Skip to content

Commit 253a733

Browse files
committed
Fix the "must highlight text in the right position" integration test
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.
1 parent f4b5ec9 commit 253a733

File tree

1 file changed

+31
-18
lines changed

1 file changed

+31
-18
lines changed

test/integration/find_spec.mjs

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,35 +39,48 @@ describe("find bar", () => {
3939
it("must highlight text in the right position", async () => {
4040
await Promise.all(
4141
pages.map(async ([browserName, page]) => {
42+
// Highlight all occurrences of the letter A (case insensitive).
4243
await page.click("#viewFind");
4344
await page.waitForSelector("#viewFind", { hidden: false });
4445
await page.type("#findInput", "a");
4546
await page.click("#findHighlightAll");
4647
await page.waitForSelector(".textLayer .highlight");
47-
// The PDF has the text "AB BA" in a monospace font.
48-
// Make sure we have the right number of highlighted divs.
48+
49+
// The PDF file contains the text 'AB BA' in a monospace font on a
50+
// single line. Check if the two occurrences of A are highlighted.
4951
const highlights = await page.$$(".textLayer .highlight");
5052
expect(highlights.length).withContext(`In ${browserName}`).toEqual(2);
51-
const glyphWidth = 15.98; // From the PDF.
52-
const pageDiv = await page.$(".page canvas");
53-
const pageBox = await pageDiv.boundingBox();
53+
54+
// Normalize the highlight's height. The font data in the PDF sets the
55+
// size of the glyphs (and therefore the size of the highlights), but
56+
// the viewer applies extra padding to them. For the comparison we
57+
// therefore use the unpadded, glyph-sized parent element's height.
58+
const parentSpan = (await highlights[0].$$("xpath/.."))[0];
59+
const parentBox = await parentSpan.boundingBox();
5460
const firstA = await highlights[0].boundingBox();
5561
const secondA = await highlights[1].boundingBox();
56-
// Subtract the page offset from the text bounding boxes;
57-
firstA.x -= pageBox.x;
58-
firstA.y -= pageBox.y;
59-
secondA.x -= pageBox.x;
60-
secondA.y -= pageBox.y;
61-
// They should be on the same line.
62+
firstA.height = parentBox.height;
63+
secondA.height = parentBox.height;
64+
65+
// Check if the vertical position of the highlights is correct. Both
66+
// should be on a single line.
6267
expect(firstA.y).withContext(`In ${browserName}`).toEqual(secondA.y);
68+
69+
// Check if the height of the two highlights is correct. Both should
70+
// match the font size.
6371
const fontSize = 26.66; // From the PDF.
64-
// The highlighted text has more padding.
65-
fuzzyMatch(firstA.height, fontSize + 5, browserName);
66-
fuzzyMatch(secondA.height, fontSize + 5, browserName);
67-
const expectedFirstAX = 28;
68-
fuzzyMatch(firstA.x, expectedFirstAX, browserName);
69-
// The second 'A' should be 4 glyphs widths from the first.
70-
fuzzyMatch(secondA.x, expectedFirstAX + glyphWidth * 4, browserName);
72+
fuzzyMatch(firstA.height, fontSize, browserName);
73+
fuzzyMatch(secondA.height, fontSize, browserName);
74+
75+
// Check if the horizontal position of the highlights is correct. The
76+
// second occurrence should be four glyph widths (three letters and
77+
// one space) away from the first occurrence.
78+
const pageDiv = await page.$(".page canvas");
79+
const pageBox = await pageDiv.boundingBox();
80+
const expectedFirstAX = 30; // From the PDF.
81+
const glyphWidth = 15.98; // From the PDF.
82+
fuzzyMatch(firstA.x, pageBox.x + expectedFirstAX, browserName);
83+
fuzzyMatch(secondA.x, firstA.x + glyphWidth * 4, browserName);
7184
})
7285
);
7386
});

0 commit comments

Comments
 (0)