Skip to content

Commit

Permalink
Fix flickering on text selection
Browse files Browse the repository at this point in the history
When seleciting on a touch screen device, whenever the finger moves to a
blank area (so over `div.textLayer` directly rather than on a `<span>`),
the selection jumps to include all the text between the beginning of the
.textLayer and the selection side that is not being moved.

The existing selection flickering fix when using the mouse cannot be
trivially re-used on mobile, because when modifying a selection on
a touchscreen device Firefox will not emit any pointer event (and
Chrome will emit them inconsistently). Instead, we have to listen to the
'selectionchange' event.

The fix is different in Firefox and Chrome:
- on Firefox, we have to make sure that, when modifying the selection,
  hovering on blank areas will hover on the .endOfContent element
  rather than on the .textLayer element. This is done by adjusting the
  z-indexes so that .endOfContent is above .textLayer.
- on Chrome, hovering on blank areas needs to trigger hovering on an
  element that is either immediately after (or immediately before,
  depending on which side of the selection the user is moving) the
  currently selected text. This is done by moving the .endOfContent
  element around between the correct `<span>`s in the text layer.

The new anti-flickering code is also used when selecting using a mouse:
the improvement in Firefox is only observable on multi-page selection,
while in Chrome it also affects selection within a single page.

After this commit, the `z-index`es inside .textLayer are as follows:
- .endOfContent has `z-index: 0`
- everything else has `z-index: 1`
  - except for .markedContent, which have `z-index: 0`
    and their contents have `z-index: 1`.

`.textLayer` has an explicit `z-index: 0` to introduce a new stacking context,
so that its contents are not drawn on top of `.annotationLayer`.
  • Loading branch information
nicolo-ribaudo committed May 7, 2024
1 parent 7290faf commit 75d9741
Show file tree
Hide file tree
Showing 7 changed files with 467 additions and 59 deletions.
1 change: 1 addition & 0 deletions test/integration-boot.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ async function runTests(results) {
"scripting_spec.mjs",
"stamp_editor_spec.mjs",
"text_field_spec.mjs",
"text_layer_spec.mjs",
"viewer_spec.mjs",
],
});
Expand Down
22 changes: 1 addition & 21 deletions test/integration/highlight_editor_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
getEditorSelector,
getFirstSerialized,
getSerialized,
getSpanRectFromText,
kbBigMoveLeft,
kbBigMoveUp,
kbFocusNext,
Expand Down Expand Up @@ -49,27 +50,6 @@ const getXY = (page, selector) =>
return `${bbox.x}::${bbox.y}`;
}, selector);

const getSpanRectFromText = async (page, pageNumber, text) => {
await page.waitForSelector(
`.page[data-page-number="${pageNumber}"] > .textLayer .endOfContent`
);
return page.evaluate(
(number, content) => {
for (const el of document.querySelectorAll(
`.page[data-page-number="${number}"] > .textLayer > span`
)) {
if (el.textContent === content) {
const { x, y, width, height } = el.getBoundingClientRect();
return { x, y, width, height };
}
}
return null;
},
pageNumber,
text
);
};

describe("Highlight Editor", () => {
describe("Editor must be removed without exception", () => {
let pages;
Expand Down
22 changes: 22 additions & 0 deletions test/integration/test_utils.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,27 @@ function getSelectedEditors(page) {
});
}

async function getSpanRectFromText(page, pageNumber, text) {
await page.waitForSelector(
`.page[data-page-number="${pageNumber}"] > .textLayer .endOfContent`
);
return page.evaluate(
(number, content) => {
for (const el of document.querySelectorAll(
`.page[data-page-number="${number}"] > .textLayer > span`
)) {
if (el.textContent === content) {
const { x, y, width, height } = el.getBoundingClientRect();
return { x, y, width, height };
}
}
return null;
},
pageNumber,
text
);
}

async function waitForEvent(page, eventName, timeout = 5000) {
const handle = await page.evaluateHandle(
(name, timeOut) => {
Expand Down Expand Up @@ -570,6 +591,7 @@ export {
getSelectedEditors,
getSelector,
getSerialized,
getSpanRectFromText,
hover,
kbBigMoveDown,
kbBigMoveLeft,
Expand Down
293 changes: 293 additions & 0 deletions test/integration/text_layer_spec.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
/* Copyright 2024 Mozilla Foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { closePages, getSpanRectFromText, loadAndWait } from "./test_utils.mjs";
import { startBrowser } from "../test.mjs";

describe("Text layer", () => {
describe("Text selection", () => {
// page.mouse.move(x, y, { steps: ... }) doesn't work in Firefox, because
// puppeteer will send fractional intermediate positions and Firefox doesn't
// support them. Use this function to round each intermediate position to an
// integer.
async function moveInSteps(page, from, to, steps) {
const deltaX = to.x - from.x;
const deltaY = to.y - from.y;
for (let i = 0; i <= steps; i++) {
const x = Math.round(from.x + (deltaX * i) / steps);
const y = Math.round(from.y + (deltaY * i) / steps);
await page.mouse.move(x, y);
}
}

function middlePosition(rect) {
return { x: rect.x + rect.width / 2, y: rect.y + rect.height / 2 };
}

function middleLeftPosition(rect) {
return { x: rect.x + 1, y: rect.y + rect.height / 2 };
}

function belowEndPosition(rect) {
return { x: rect.x + rect.width, y: rect.y + rect.height * 1.5 };
}

beforeAll(() => {
jasmine.addAsyncMatchers({
// Check that a page has a selection containing the given text, with
// some tolerance for extra characters before/after.
toHaveRoughlySelected({ pp }) {
return {
async compare(page, expected) {
const TOLERANCE = 10;

const actual = await page.evaluate(() =>
// We need to normalize EOL for Windows
window.getSelection().toString().replaceAll("\r\n", "\n")
);

let start, end;
if (expected instanceof RegExp) {
const match = expected.exec(actual);
start = -1;
if (match) {
start = match.index;
end = start + match[0].length;
}
} else {
start = actual.indexOf(expected);
if (start !== -1) {
end = start + expected.length;
}
}

const pass =
start !== -1 &&
start < TOLERANCE &&
end > actual.length - TOLERANCE;

return {
pass,
message: `Expected ${pp(
actual.length > 200
? actual.slice(0, 100) + "[...]" + actual.slice(-100)
: actual
)} to ${pass ? "not " : ""}roughly match ${pp(expected)}.`,
};
},
};
},
});
});

describe("using mouse", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"tracemonkey.pdf",
`.page[data-page-number = "1"] .endOfContent`
);
});
afterAll(async () => {
await closePages(pages);
});

it("doesn't jump when hovering on an empty area", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const [positionStart, positionEnd] = await Promise.all([
getSpanRectFromText(
page,
1,
"(frequently executed) bytecode sequences, records"
).then(middlePosition),
getSpanRectFromText(
page,
1,
"them, and compiles them to fast native code. We call such a se-"
).then(belowEndPosition),
]);

await page.mouse.move(positionStart.x, positionStart.y);
await page.mouse.down();
await moveInSteps(page, positionStart, positionEnd, 20);
await page.mouse.up();

await expectAsync(page)
.withContext(`In ${browserName}`)
.toHaveRoughlySelected(
"code sequences, records\n" +
"them, and compiles them to fast native code. We call suc"
);
})
);
});

it("doesn't jump when hovering on an empty area (multi-page)", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const scrollTarget = await getSpanRectFromText(
page,
1,
"Unlike method-based dynamic compilers, our dynamic com-"
);
await page.evaluate(top => {
document.getElementById("viewerContainer").scrollTop = top;
}, scrollTarget.y - 50);

const [
positionStartPage1,
positionEndPage1,
positionStartPage2,
positionEndPage2,
] = await Promise.all([
getSpanRectFromText(
page,
1,
"Each compiled trace covers one path through the program with"
).then(middlePosition),
getSpanRectFromText(
page,
1,
"or that the same types will occur in subsequent loop iterations."
).then(middlePosition),
getSpanRectFromText(
page,
2,
"Hence, recording and compiling a trace"
).then(middlePosition),
getSpanRectFromText(
page,
2,
"cache. Alternatively, the VM could simply stop tracing, and give up"
).then(belowEndPosition),
]);

await page.mouse.move(positionStartPage1.x, positionStartPage1.y);
await page.mouse.down();

await moveInSteps(page, positionStartPage1, positionEndPage1, 20);
await moveInSteps(page, positionEndPage1, positionStartPage2, 20);

await expectAsync(page)
.withContext(`In ${browserName}, first selection`)
.toHaveRoughlySelected(
/path through the program .*Hence, recording a/s
);

await moveInSteps(page, positionStartPage2, positionEndPage2, 20);
await page.mouse.up();

await expectAsync(page)
.withContext(`In ${browserName}, second selection`)
.toHaveRoughlySelected(
/path through.*Hence, recording and .* tracing, and give/s
);
})
);
});
});

describe("using selection carets", () => {
let browser;
let page;

beforeAll(async () => {
// Chrome does not support simulating caret-based selection, so this
// test only runs in Firefox.
browser = await startBrowser({
browserName: "firefox",
startUrl: "",
extraPrefsFirefox: {
"layout.accessiblecaret.enabled": true,
"layout.accessiblecaret.hide_carets_for_mouse_input": false,
},
});
page = await browser.newPage();
await page.goto(
`${global.integrationBaseUrl}?file=/test/pdfs/tracemonkey.pdf#zoom=page-fit`
);
await page.bringToFront();
await page.waitForSelector(
`.page[data-page-number = "1"] .endOfContent`,
{ timeout: 0 }
);
});
afterAll(async () => {
await browser.close();
});

it("doesn't jump when moving selection", async () => {
const [initialStart, initialEnd, finalEnd] = await Promise.all([
getSpanRectFromText(
page,
1,
"(frequently executed) bytecode sequences, records"
).then(middleLeftPosition),
getSpanRectFromText(
page,
1,
"(frequently executed) bytecode sequences, records"
).then(middlePosition),
getSpanRectFromText(
page,
1,
"them, and compiles them to fast native code. We call such a se-"
).then(belowEndPosition),
]);

await page.mouse.move(initialStart.x, initialStart.y);
await page.mouse.down();
await moveInSteps(page, initialStart, initialEnd, 20);
await page.mouse.up();

await expectAsync(page)
.withContext(`first selection`)
.toHaveRoughlySelected("(frequently executed) byt");

const initialCaretPos = {
x: initialEnd.x,
y: initialEnd.y + 10,
};
const intermediateCaretPos = {
x: finalEnd.x,
y: finalEnd.y + 5,
};
const finalCaretPos = {
x: finalEnd.x + 20,
y: finalEnd.y + 5,
};

await page.mouse.move(initialCaretPos.x, initialCaretPos.y);
await page.mouse.down();
await moveInSteps(page, initialCaretPos, intermediateCaretPos, 20);
await page.mouse.up();

await expectAsync(page)
.withContext(`second selection`)
.toHaveRoughlySelected(/\(frequently .* We call such a se/s);

await page.mouse.down();
await moveInSteps(page, intermediateCaretPos, finalCaretPos, 20);
await page.mouse.up();

await expectAsync(page)
.withContext(`third selection`)
.toHaveRoughlySelected(/\(frequently .* We call such a se/s);
});
});
});
});
Loading

0 comments on commit 75d9741

Please sign in to comment.