Skip to content

Commit

Permalink
Fix flickering on Chrome desktop, and when there is .markedContent
Browse files Browse the repository at this point in the history
This commit removes the existing anti-flickering implementation
for Chrome desktop, and replaces it with the same one used for
mobile. This prevents much more flickering also on desktop.

This commit also ensures that also nested `<span>` elements inside
.textLayer get a `z-index: 1`, while leaving .markedContent elements
at `z-index: 0` (and the `<span>`s they contain will have `z-index: 1`).
In Chrome, .endOfContent can now be moved right after the selected
`<span>` even if it's inside a .markedContent element, removing
flickering over the .markedContent section.
  • Loading branch information
nicolo-ribaudo committed Apr 16, 2024
1 parent 67da5fd commit 7d096ec
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 33 deletions.
8 changes: 4 additions & 4 deletions web/text_layer_builder.css
Expand Up @@ -37,6 +37,10 @@
transform-origin: 0% 0%;
}

span:not(.markedContent) {
z-index: 1;
}

/* Only necessary in Google Chrome, see issue 14205, and most unfortunately
* the problem doesn't show up in "text" reference tests. */
/*#if !MOZCENTRAL*/
Expand Down Expand Up @@ -116,8 +120,4 @@
top: 0;
}
}

> * {
z-index: 1;
}
}
57 changes: 28 additions & 29 deletions web/text_layer_builder.js
Expand Up @@ -193,27 +193,14 @@ class TextLayerBuilder {

div.addEventListener("mousedown", evt => {
selectingThroughMouse = true;

if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
isFirefox ??=
(typeof PDFJSDev === "undefined" || !PDFJSDev.test("CHROME")) &&
getComputedStyle(end).getPropertyValue("-moz-user-select") === "none";
// On non-Firefox browsers, the selection will feel better if the height
// of the `endOfContent` div is adjusted to start at mouse click
// location. This avoids flickering when the selection moves up.
// However it does not work when selection is started on empty space.
if (!isFirefox && evt.target !== div) {
const divBounds = div.getBoundingClientRect();
const r = Math.max(0, (evt.pageY - divBounds.top) / divBounds.height);
end.style.top = (r * 100).toFixed(2) + "%";
}
}
end.classList.add("active");
});

const reset = () => {
if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
end.style.top = "";
div.appendChild(end);

Check failure on line 201 in web/text_layer_builder.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

Prefer `Node#append()` over `Node#appendChild()`
end.style.width = "";
end.style.height = "";
}
end.classList.remove("active");
selectingThroughMouse = false;
Expand Down Expand Up @@ -242,8 +229,19 @@ class TextLayerBuilder {
document.addEventListener(
"selectionchange",
() => {
if (selectingThroughMouse) {
return;
if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
isFirefox ??=
(typeof PDFJSDev === "undefined" || !PDFJSDev.test("CHROME")) &&
getComputedStyle(end).getPropertyValue("-moz-user-select") ===
"none";

if (isFirefox && selectingThroughMouse) {
return;
}
} else {
if (selectingThroughMouse) {

Check failure on line 242 in web/text_layer_builder.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

Unexpected if as the only statement in an else block
return;
}
}

const selection = document.getSelection();
Expand All @@ -259,10 +257,6 @@ class TextLayerBuilder {
}

if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
isFirefox ??=
(typeof PDFJSDev === "undefined" || !PDFJSDev.test("CHROME")) &&
getComputedStyle(end).getPropertyValue("-moz-user-select") ===
"none";
// In non-Firefox browsers, when hovering over an empty space (thus, on

Check failure on line 260 in web/text_layer_builder.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

This line has a comment length of 81. Maximum allowed is 80
// .endOfContent), the selection will expand to cover all the text
// between the current selection and .endOfContent. By moving
Expand All @@ -283,19 +277,24 @@ class TextLayerBuilder {
anchor = anchor.parentNode;
}
// Make sure that we are always only moving .endOfContent to somewhere

Check failure on line 279 in web/text_layer_builder.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

This line has a comment length of 82. Maximum allowed is 80
// directly in .textLayer, and not inside nested spans.
anchor = anchor.closest(".textLayer > span");
anchor.parentElement.insertBefore(
end,
modifyStart ? anchor : anchor.nextSibling
);
// inside this .textLayer, in case of multi-page selections.
const parentTextLayer = anchor.parentElement.closest(".textLayer");
if (parentTextLayer === div) {
end.style.width = div.style.width;
end.style.height = div.style.height;
anchor.parentElement.insertBefore(
end,
modifyStart ? anchor : anchor.nextSibling
);
}

prevRange = range.cloneRange();
}
}

end.classList.add("active");
},
this.#selectionChangeAbortController
{ signal: this.#selectionChangeAbortController.signal }
);

div.addEventListener("copy", event => {
Expand Down

0 comments on commit 7d096ec

Please sign in to comment.