Skip to content

Commit

Permalink
Disable link annotations during text selection
Browse files Browse the repository at this point in the history 
When selecting text, hovering over an element
causes all the text between (according the the dom
order) the current selection and that element to
be selected.

This means that when, while selecting, the cursor
moves over a link, all the text in the page gets
selected. Setting `user-select: none` on the link
annotations would improve the situation, but it
still makes it impossible to extend the selection
within a link without using Shift+arrows keys on
the keyboard.

This commit fixes the problem by setting
`pointer-events: none` on the `<section>`s in the
annotation layer while selecting some text. This
way, they are ignored for hit-testing and do not
affect selection.

It is still impossible to _start_ a selection
inside a link, as the link text is covered by the
link annotation.

Fixes mozilla#18266
  • Loading branch information
@nicolo-ribaudo
nicolo-ribaudo committed Jul 23, 2024
1 parent 98e7727 commit 64a0e59
Show file tree
Hide file tree
Showing 5 changed files with 287 additions and 99 deletions.
2 changes: 1 addition & 1 deletion test/integration/test_utils.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ async function getSpanRectFromText(page, pageNumber, text) {
return page.evaluate(
(number, content) => {
for (const el of document.querySelectorAll(
`.page[data-page-number="${number}"] > .textLayer > span`
`.page[data-page-number="${number}"] > .textLayer span:not(:has(> span))`
)) {
if (el.textContent === content) {
const { x, y, width, height } = el.getBoundingClientRect();
Expand Down
340 changes: 249 additions & 91 deletions test/integration/text_layer_spec.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/

import {
awaitPromise,
closePages,
closeSinglePage,
getSpanRectFromText,
Expand Down Expand Up @@ -98,111 +99,268 @@ describe("Text layer", () => {
});

describe("using mouse", () => {
let pages;
describe("doesn't jump when hovering on an empty area", () => {
let pages;

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

beforeAll(async () => {
pages = await loadAndWait(
"tracemonkey.pdf",
`.page[data-page-number = "1"] .endOfContent`
);
});
afterAll(async () => {
await closePages(pages);
});
it("in a single page", 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", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const [positionStart, positionEnd] = await Promise.all([
getSpanRectFromText(
it("across multiple pages", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const scrollTarget = await 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"
"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
);
})
);
});
});

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),
]);
describe("when selecting over a link", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"annotation-link-text-popup.pdf",
`.page[data-page-number = "1"] .endOfContent`
);
});
afterAll(async () => {
await closePages(pages);
});
afterEach(() =>
Promise.all(
pages.map(([_, page]) =>
page.evaluate(() => window.getSelection().removeAllRanges())
)
)
);

await page.mouse.move(positionStartPage1.x, positionStartPage1.y);
await page.mouse.down();
function waitForClick(page, selector, timeout) {
return page.evaluateHandle(
(sel, timeoutDelay) => {
const element = document.querySelector(sel);
const timeoutSignal = AbortSignal.timeout(timeoutDelay);
return [
new Promise(resolve => {
timeoutSignal.addEventListener(
"abort",
() => resolve(false),
{ once: true }
);
element.addEventListener(
"click",
e => {
e.preventDefault();
resolve(true);
},
{ once: true, signal: timeoutSignal }
);
}),
];
},
selector,
timeout
);
}

it("allows selecting within the link", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const [positionStart, positionEnd] = await Promise.all([
getSpanRectFromText(page, 1, "Link").then(middleLeftPosition),
getSpanRectFromText(page, 1, "mozilla.org").then(
middlePosition
),
]);

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("Link\nmozil");
})
);
});

await moveInSteps(page, positionStartPage1, positionEndPage1, 20);
await moveInSteps(page, positionEndPage1, positionStartPage2, 20);
it("allows selecting within the link when going backwards", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const [positionStart, positionEnd] = await Promise.all([
getSpanRectFromText(page, 1, "Text").then(middlePosition),
getSpanRectFromText(page, 1, "mozilla.org").then(
middlePosition
),
]);

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("a.org\nTe");
})
);
});

await expectAsync(page)
.withContext(`In ${browserName}, first selection`)
.toHaveRoughlySelected(
/path through the program .*Hence, recording a/s
it("allows clicking the link after selecting", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const [positionStart, positionEnd] = await Promise.all([
getSpanRectFromText(page, 1, "Link").then(middleLeftPosition),
getSpanRectFromText(page, 1, "mozilla.org").then(
middlePosition
),
]);

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

const clickPromiseHandle = await waitForClick(
page,
"#pdfjs_internal_id_8R",
1000
);

await moveInSteps(page, positionStartPage2, positionEndPage2, 20);
await page.mouse.up();
await page.mouse.click(positionEnd.x, positionEnd.y);

const clicked = await awaitPromise(clickPromiseHandle);
expect(clicked).toBeTrue();
})
);
});

await expectAsync(page)
.withContext(`In ${browserName}, second selection`)
.toHaveRoughlySelected(
/path through.*Hence, recording and .* tracing, and give/s
it("allows clicking the link after changing selection with the keyboard", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const [positionStart, positionEnd] = await Promise.all([
getSpanRectFromText(page, 1, "Link").then(middleLeftPosition),
getSpanRectFromText(page, 1, "mozilla.org").then(
middlePosition
),
]);

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

await page.keyboard.down("Shift");
await page.keyboard.press("ArrowRight");
await page.keyboard.up("Shift");

const clickPromiseHandle = await waitForClick(
page,
"#pdfjs_internal_id_8R",
1000
);
})
);

await page.mouse.click(positionEnd.x, positionEnd.y);

const clicked = await awaitPromise(clickPromiseHandle);
expect(clicked).toBeTrue();
})
);
});
});
});

Expand Down
4 changes: 4 additions & 0 deletions web/annotation_layer_builder.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@
}
}

.textLayer.selecting ~ & section {
pointer-events: none;
}

:is(.linkAnnotation, .buttonWidgetAnnotation.pushButton) > a {
position: absolute;
font-size: 1em;
Expand Down
Loading

0 comments on commit 64a0e59

Please sign in to comment.