Skip to content

Commit f01e4d4

Browse files
committed
fix: prevent search highlights from interfering with drag-selection
1 parent 62ac1b8 commit f01e4d4

2 files changed

Lines changed: 77 additions & 0 deletions

File tree

test/integration/text_layer_spec.mjs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,80 @@ describe("Text layer", () => {
432432
);
433433
});
434434
});
435+
436+
describe("when selecting text with find highlights active", () => {
437+
let pages;
438+
439+
beforeEach(async () => {
440+
pages = await loadAndWait("find_all.pdf", ".textLayer", 100);
441+
});
442+
443+
afterEach(async () => {
444+
await closePages(pages);
445+
});
446+
447+
it("doesn't jump when selection anchor is inside a highlight element", async () => {
448+
await Promise.all(
449+
pages.map(async ([browserName, page]) => {
450+
// Highlight all occurrences of the letter A (case insensitive).
451+
await page.click("#viewFindButton");
452+
await page.waitForSelector("#findInput", { visible: true });
453+
await page.type("#findInput", "a");
454+
await page.click("#findHighlightAll + label");
455+
await page.waitForSelector(".textLayer .highlight");
456+
457+
// find_all.pdf contains 'AB BA' in a monospace font. These are
458+
// the glyph metrics at 100% zoom, extracted from the PDF.
459+
const glyphWidth = 15.98;
460+
const expectedFirstAX = 30;
461+
462+
// Compute the drag coordinates to select exactly "AB". The
463+
// horizontal positions use the page origin and PDF glyph
464+
// metrics; the vertical center comes from the highlight.
465+
const pageDiv = await page.$(".page canvas");
466+
const pageBox = await pageDiv.boundingBox();
467+
const firstHighlight = await page.$(".textLayer .highlight");
468+
const highlightBox = await firstHighlight.boundingBox();
469+
470+
// Drag from beginning of first 'A' to end of second 'B'
471+
const aStart = pageBox.x + expectedFirstAX;
472+
const startY = Math.round(
473+
highlightBox.y + highlightBox.height / 2
474+
);
475+
const bEnd = Math.round(aStart + glyphWidth * 2);
476+
477+
await page.mouse.move(aStart, startY);
478+
await page.mouse.down();
479+
await moveInSteps(
480+
page,
481+
{ x: aStart, y: startY },
482+
{ x: bEnd, y: startY },
483+
20
484+
);
485+
await page.mouse.up();
486+
487+
const selection = await page.evaluate(() =>
488+
window.getSelection().toString()
489+
);
490+
expect(selection).withContext(`In ${browserName}`).toEqual("AB");
491+
492+
// The selectionchange handler in TextLayerBuilder walks up
493+
// from .highlight to its parent span before placing
494+
// endOfContent (see text_layer_builder.js). Without that
495+
// fix, endOfContent would be inserted inside the text span
496+
// (as a sibling of the .highlight) instead of as a direct
497+
// child of .textLayer. Verify the correct DOM structure.
498+
const endOfContentIsDirectChild = await page.evaluate(() => {
499+
const eoc = document.querySelector(".textLayer .endOfContent");
500+
return eoc?.parentElement?.classList.contains("textLayer");
501+
});
502+
expect(endOfContentIsDirectChild)
503+
.withContext(`In ${browserName}`)
504+
.toBeTrue();
505+
})
506+
);
507+
});
508+
});
435509
});
436510

437511
describe("using selection carets", () => {

web/text_layer_builder.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,9 @@ class TextLayerBuilder {
307307
if (anchor.nodeType === Node.TEXT_NODE) {
308308
anchor = anchor.parentNode;
309309
}
310+
if (anchor.classList?.contains("highlight")) {
311+
anchor = anchor.parentNode;
312+
}
310313
if (!modifyStart && range.endOffset === 0) {
311314
do {
312315
while (!anchor.previousSibling) {

0 commit comments

Comments
 (0)