Skip to content

Commit

Permalink
Fix the "must highlight text in the right position" integration test
Browse files Browse the repository at this point in the history
This commit implements the following improvements for the test:

- Replace the hardcoded highlight padding with a dynamic calculation. It
  turns out that the amount of padding can differ per system, possibly
  due to e.g. local (font) settings, which could cause the test to fail.
  The test now no longer implicitly depends on the environment.
- Remove the page offset subtraction. It's only needed for one assertion,
  and we can simply add the page offset there once.
- Improve the "magic" numbers in the test. The number 5 is removed now
  that the padding calculation made it obsolete, the number 28 is changed
  to 30 because that's the actual value in the PDF data and the number 4
  is explained a bit more to state that the space also counts as a glyph.
- Annotate the steps and checks to improve readability of the test and
  to explain why certain calculations have to be performed.
  • Loading branch information
timvandermeij committed Apr 9, 2024
1 parent a208d6b commit bcd2760
Showing 1 changed file with 32 additions and 18 deletions.
50 changes: 32 additions & 18 deletions test/integration/find_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -39,35 +39,49 @@ describe("find bar", () => {
it("must highlight text in the right position", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
// Highlight all occurrences of the letter A (case insensitive).
await page.click("#viewFind");
await page.waitForSelector("#viewFind", { hidden: false });
await page.type("#findInput", "a");
await page.click("#findHighlightAll");
await page.waitForSelector(".textLayer .highlight");
// The PDF has the text "AB BA" in a monospace font.
// Make sure we have the right number of highlighted divs.

// The PDF file contains the text 'AB BA' in a monospace font on a
// single line. Check if the two occurrences of A are highlighted.
const highlights = await page.$$(".textLayer .highlight");
expect(highlights.length).withContext(`In ${browserName}`).toEqual(2);
const glyphWidth = 15.98; // From the PDF.
const pageDiv = await page.$(".page canvas");
const pageBox = await pageDiv.boundingBox();

// Calculate the dimensions of the highlights. The font data in the
// PDF describes the size of the glyphs (and therefore the size of the
// highlights), but the viewer applies extra padding to them. For the
// comparison we therefore subtract the padding, which we calculate
// using the unpadded parent element, from the highlight's height.
const parentSpan = (await highlights[0].$$("xpath/.."))[0];
const parentBox = await parentSpan.boundingBox();
const firstA = await highlights[0].boundingBox();
const secondA = await highlights[1].boundingBox();
// Subtract the page offset from the text bounding boxes;
firstA.x -= pageBox.x;
firstA.y -= pageBox.y;
secondA.x -= pageBox.x;
secondA.y -= pageBox.y;
// They should be on the same line.
firstA.height -= firstA.height - parentBox.height;
secondA.height -= secondA.height - parentBox.height;

// Check if the vertical position of the highlights is correct. Both
// should be on a single line.
expect(firstA.y).withContext(`In ${browserName}`).toEqual(secondA.y);

// Check if the height of the two highlights is correct. Both should
// match the font size.
const fontSize = 26.66; // From the PDF.
// The highlighted text has more padding.
fuzzyMatch(firstA.height, fontSize + 5, browserName);
fuzzyMatch(secondA.height, fontSize + 5, browserName);
const expectedFirstAX = 28;
fuzzyMatch(firstA.x, expectedFirstAX, browserName);
// The second 'A' should be 4 glyphs widths from the first.
fuzzyMatch(secondA.x, expectedFirstAX + glyphWidth * 4, browserName);
fuzzyMatch(firstA.height, fontSize, browserName);
fuzzyMatch(secondA.height, fontSize, browserName);

// Check if the horizontal position of the highlights is correct. The
// second occurrence should be four glyph widths (three letters and
// one space) away from the first occurrence.
const pageDiv = await page.$(".page canvas");
const pageBox = await pageDiv.boundingBox();
const expectedFirstAX = 30; // From the PDF.
const glyphWidth = 15.98; // From the PDF.
fuzzyMatch(firstA.x, pageBox.x + expectedFirstAX, browserName);
fuzzyMatch(secondA.x, firstA.x + glyphWidth * 4, browserName);
})
);
});
Expand Down

0 comments on commit bcd2760

Please sign in to comment.