Skip to content

Commit da5b681

Browse files
[chrome] Fix text selection with .markedContent
The current text layer approach based on absolutely positioned `<span>` elements by default causes flickering with text selection, and we have browser-specific workarounds to solve that. In Chrome, the workaround involves moving the `.endOfContent` element to right after the last element that contains some selected content. This works well in simple PDFs, but breaks when we have `span.markedContent` elements. Given a text layer structure like the following, rendered as four consecutive lines: ```html <span class="markedContent"> <br> <span>development enter the construction phase (estimated at around</span> </span> <span class="markedContent"> <br> <span>300 MEUR).</span> </span> <span class="markedContent"> <br> <span>Kreate's EBITA increased to 2.8 MEUR (Q4'23: 2.7 MEUR) and the</span> </span> <span class="markedContent"> <br> <span>margin rose to 3.7% (Q4'23: 3.4%). However, profitability was</span> </span> ``` when starting to select from inside the first line and dragging down to the empty space after the second line, Chrome will anchor the selection at the beginning of either the `<br>` or the `<span>` inside the last `.markedContent`, depending on whether the selection is in "per-character mode" (i.e. click and drag) or "per-word mode" (i.e. double click and drag). This causes us to insert the `.endOfContent` element in the wrong place (one element too far), which causes one more line to be selected, which triggers another `"selecctionchange"` event, which causes us to move `.endOfContent` again, and so on, looping until when the whole page is selected. This commit fixes the issue by making sure that when the end of the selection range points to the _begining_ of an element, we walk back the dom finding the first non-empty element, and attatch `.endOfContent` to the end of that.
1 parent 72feb4c commit da5b681

4 files changed

Lines changed: 107 additions & 0 deletions

File tree

test/integration/text_layer_spec.mjs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,104 @@ describe("Text layer", () => {
217217
});
218218
});
219219

220+
describe("doesn't jump when hovering on an empty area, with .markedContent", () => {
221+
let pages;
222+
223+
beforeAll(async () => {
224+
pages = await loadAndWait(
225+
"chrome-text-selection-markedContent.pdf",
226+
`.page[data-page-number = "1"] .endOfContent`
227+
);
228+
});
229+
afterAll(async () => {
230+
await closePages(pages);
231+
});
232+
233+
it("in per-character selection mode", async () => {
234+
await Promise.all(
235+
pages.map(async ([browserName, page]) => {
236+
const [positionStart, positionEnd] = await Promise.all([
237+
getSpanRectFromText(
238+
page,
239+
1,
240+
"strengthen in the coming quarters as the railway projects under"
241+
).then(middlePosition),
242+
getSpanRectFromText(
243+
page,
244+
1,
245+
"development enter the construction phase (estimated at around"
246+
).then(belowEndPosition),
247+
]);
248+
249+
await page.mouse.move(positionStart.x, positionStart.y);
250+
await page.mouse.down();
251+
await moveInSteps(page, positionStart, positionEnd, 20);
252+
await page.mouse.up();
253+
254+
await expectAsync(page)
255+
.withContext(`In ${browserName}`)
256+
.toHaveRoughlySelected(
257+
"rs as the railway projects under\n" +
258+
"development enter the construction phase (estimated at "
259+
);
260+
})
261+
);
262+
});
263+
264+
it("in per-word selection mode", async () => {
265+
await Promise.all(
266+
pages.map(async ([browserName, page]) => {
267+
const [positionStart, positionEnd] = await Promise.all([
268+
getSpanRectFromText(
269+
page,
270+
1,
271+
"strengthen in the coming quarters as the railway projects under"
272+
).then(middlePosition),
273+
getSpanRectFromText(
274+
page,
275+
1,
276+
"development enter the construction phase (estimated at around"
277+
).then(belowEndPosition),
278+
]);
279+
280+
if (browserName !== "firefox") {
281+
await page.mouse.move(positionStart.x, positionStart.y);
282+
await page.mouse.down({ clickCount: 1 });
283+
await page.mouse.up({ clickCount: 1 });
284+
await page.mouse.down({ clickCount: 2 });
285+
} else {
286+
// When running tests with Firefox we use WebDriver BiDi, for
287+
// which puppeteer doesn't support emulating "double click and
288+
// hold". We need to manually dispatch an action through the
289+
// protocol.
290+
// See https://github.com/puppeteer/puppeteer/issues/13745.
291+
await page.mainFrame().browsingContext.performActions([
292+
{
293+
type: "pointer",
294+
id: "__puppeteer_mouse",
295+
actions: [
296+
{ type: "pointerMove", ...positionStart },
297+
{ type: "pointerDown", button: 0 },
298+
{ type: "pointerUp", button: 0 },
299+
{ type: "pointerDown", button: 0 },
300+
],
301+
},
302+
]);
303+
}
304+
await moveInSteps(page, positionStart, positionEnd, 20);
305+
await page.mouse.up();
306+
307+
await expectAsync(page)
308+
.withContext(`In ${browserName}`)
309+
.toHaveRoughlySelected(
310+
"quarters as the railway projects under\n" +
311+
"development enter the construction phase (estimated at around"
312+
);
313+
})
314+
);
315+
});
316+
});
317+
220318
describe("when selecting over a link", () => {
221319
let pages;
222320

test/pdfs/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,3 +718,4 @@
718718
!issue19424.pdf
719719
!issue18529.pdf
720720
!issue16742.pdf
721+
!chrome-text-selection-markedContent.pdf
134 KB
Binary file not shown.

web/text_layer_builder.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,14 @@ class TextLayerBuilder {
307307
if (anchor.nodeType === Node.TEXT_NODE) {
308308
anchor = anchor.parentNode;
309309
}
310+
if (!modifyStart && range.endOffset === 0) {
311+
do {
312+
while (!anchor.previousSibling) {
313+
anchor = anchor.parentNode;
314+
}
315+
anchor = anchor.previousSibling;
316+
} while (!anchor.childNodes.length);
317+
}
310318

311319
const parentTextLayer = anchor.parentElement?.closest(".textLayer");
312320
const endDiv = this.#textLayers.get(parentTextLayer);

0 commit comments

Comments
 (0)