Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[Editor] In caret browsing mode, allow to select in pressing shift and arrow down (bug 1881802)

Browse Source 
In implementing caret browsing mode in pdf.js, I didn't notice that selectstart isn't always triggered.
So this patch removes the use of selectstart and rely only on selectionchange.
In order to simplify the selection management, the selection code is moved in the AnnotationUIManager:
 - it simplifies the code;
 - it allows to have only one listener for selectionchange instead of having one by visible page
   for selectstart.
I had to add a delay in the integration tests for highlighting (there's a comment with an explanation),
it isn't really nice, but it's the only way I found and in real life there always is a delay between
press and release.
This commit is contained in:
Calixte Denizet 2024-02-23 18:53:58 +01:00
parent b8b8f1af66
commit 0520f2f0cb
4 changed files with 132 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
src/display/editor/annotation_editor_layer.js
View File

@ -63,16 +63,12 @@ class AnnotationEditorLayer {
#boundPointerup = this.pointerup.bind(this); #boundPointerup = this.pointerup.bind(this);
#boundPointerUpAfterSelection = this.pointerUpAfterSelection.bind(this);
#boundPointerdown = this.pointerdown.bind(this); #boundPointerdown = this.pointerdown.bind(this);
#boundTextLayerPointerDown = this.#textLayerPointerDown.bind(this); #boundTextLayerPointerDown = this.#textLayerPointerDown.bind(this);
#editorFocusTimeoutId = null; #editorFocusTimeoutId = null;
#boundSelectionStart = this.selectionStart.bind(this);
#editors = new Map(); #editors = new Map();
#hadPointerDown = false; #hadPointerDown = false;
@ -338,7 +334,6 @@ class AnnotationEditorLayer {
enableTextSelection() { enableTextSelection() {
if (this.#textLayer?.div) { if (this.#textLayer?.div) {
document.addEventListener("selectstart", this.#boundSelectionStart);
this.#textLayer.div.addEventListener( this.#textLayer.div.addEventListener(
"pointerdown", "pointerdown",
this.#boundTextLayerPointerDown this.#boundTextLayerPointerDown
@ -349,7 +344,6 @@ class AnnotationEditorLayer {
disableTextSelection() { disableTextSelection() {
if (this.#textLayer?.div) { if (this.#textLayer?.div) {
document.removeEventListener("selectstart", this.#boundSelectionStart);
this.#textLayer.div.removeEventListener( this.#textLayer.div.removeEventListener(
"pointerdown", "pointerdown",
this.#boundTextLayerPointerDown this.#boundTextLayerPointerDown
@ -686,54 +680,6 @@ class AnnotationEditorLayer {
this.#uiManager.unselect(editor); this.#uiManager.unselect(editor);
} }
/**
* SelectionChange callback.
* @param {Event} event
*/
selectionStart(event) {
if (
!this.#textLayer ||
event.target.parentElement.closest(".textLayer") !== this.#textLayer.div
) {
return;
}
if (this.#uiManager.isShiftKeyDown) {
const keyup = () => {
if (this.#uiManager.isShiftKeyDown) {
return;
}
window.removeEventListener("keyup", keyup);
window.removeEventListener("blur", keyup);
this.pointerUpAfterSelection({});
};
window.addEventListener("keyup", keyup);
window.addEventListener("blur", keyup);
} else {
this.#textLayer.div.addEventListener(
"pointerup",
this.#boundPointerUpAfterSelection,
{ once: true }
);
}
}
/**
* Called when the user releases the mouse button after having selected
* some text.
* @param {PointerEvent} event
*/
pointerUpAfterSelection(event) {
const boxes = this.#uiManager.getSelectionBoxes(this.#textLayer?.div);
if (boxes) {
this.createAndAddNewEditor(event, false, {
boxes,
});
document.getSelection().empty();
}
}
/** /**
* Pointerup callback. * Pointerup callback.
* @param {PointerEvent} event * @param {PointerEvent} event

61
src/display/editor/tools.js
View File

@ -57,11 +57,19 @@ function opacityToHex(opacity) {
class IdManager { class IdManager {
#id = 0; #id = 0;
constructor() {
if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
Object.defineProperty(this, "reset", {
value: () => (this.#id = 0),
});
}
}
/** /**
* Get a unique id. * Get a unique id.
* @returns {string} * @returns {string}
*/ */
getId() { get id() {
return `${AnnotationEditorPrefix}${this.#id++}`; return `${AnnotationEditorPrefix}${this.#id++}`;
} }
} }
@ -551,10 +559,10 @@ class AnnotationEditorUIManager {
#focusMainContainerTimeoutId = null; #focusMainContainerTimeoutId = null;
#hasSelection = false;
#highlightColors = null; #highlightColors = null;
#highlightWhenShiftUp = false;
#idManager = new IdManager(); #idManager = new IdManager();
#isEnabled = false; #isEnabled = false;
@ -780,6 +788,16 @@ class AnnotationEditorUIManager {
rotation: 0, rotation: 0,
}; };
this.isShiftKeyDown = false; this.isShiftKeyDown = false;
if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
Object.defineProperty(this, "reset", {
value: () => {
this.selectAll();
this.delete();
this.#idManager.reset();
},
});
}
} }
destroy() { destroy() {
@ -958,8 +976,7 @@ class AnnotationEditorUIManager {
#selectionChange() { #selectionChange() {
const selection = document.getSelection(); const selection = document.getSelection();
if (!selection || selection.isCollapsed) { if (!selection || selection.isCollapsed) {
if (this.#hasSelection) { if (this.#selectedTextNode) {
this.#hasSelection = false;
this.#selectedTextNode = null; this.#selectedTextNode = null;
this.#dispatchUpdateStates({ this.#dispatchUpdateStates({
hasSelectedText: false, hasSelectedText: false,
@ -977,8 +994,7 @@ class AnnotationEditorUIManager {
? anchorNode.parentElement ? anchorNode.parentElement
: anchorNode; : anchorNode;
if (!anchorElement.closest(".textLayer")) { if (!anchorElement.closest(".textLayer")) {
if (this.#hasSelection) { if (this.#selectedTextNode) {
this.#hasSelection = false;
this.#selectedTextNode = null; this.#selectedTextNode = null;
this.#dispatchUpdateStates({ this.#dispatchUpdateStates({
hasSelectedText: false, hasSelectedText: false,
@ -986,11 +1002,30 @@ class AnnotationEditorUIManager {
} }
return; return;
} }
this.#hasSelection = true;
this.#selectedTextNode = anchorNode; this.#selectedTextNode = anchorNode;
this.#dispatchUpdateStates({ this.#dispatchUpdateStates({
hasSelectedText: true, hasSelectedText: true,
}); });
if (this.#mode !== AnnotationEditorType.HIGHLIGHT) {
return;
}
this.#highlightWhenShiftUp = this.isShiftKeyDown;
if (!this.isShiftKeyDown) {
const pointerup = e => {
if (e.type === "pointerup" && e.button !== 0) {
// Do nothing on right click.
return;
}
window.removeEventListener("pointerup", pointerup);
window.removeEventListener("blur", pointerup);
if (e.type === "pointerup") {
this.highlightSelection();
}
};
window.addEventListener("pointerup", pointerup);
window.addEventListener("blur", pointerup);
}
} }
#addSelectionListener() { #addSelectionListener() {
@ -1013,6 +1048,10 @@ class AnnotationEditorUIManager {
blur() { blur() {
this.isShiftKeyDown = false; this.isShiftKeyDown = false;
if (this.#highlightWhenShiftUp) {
this.#highlightWhenShiftUp = false;
this.highlightSelection();
}
if (!this.hasSelection) { if (!this.hasSelection) {
return; return;
} }
@ -1199,6 +1238,10 @@ class AnnotationEditorUIManager {
keyup(event) { keyup(event) {
if (this.isShiftKeyDown && event.key === "Shift") { if (this.isShiftKeyDown && event.key === "Shift") {
this.isShiftKeyDown = false; this.isShiftKeyDown = false;
if (this.#highlightWhenShiftUp) {
this.#highlightWhenShiftUp = false;
this.highlightSelection();
}
} }
} }
@ -1287,7 +1330,7 @@ class AnnotationEditorUIManager {
* @returns {string} * @returns {string}
*/ */
getId() { getId() {
return this.#idManager.getId(); return this.#idManager.id;
} }
get currentLayer() { get currentLayer() {

94
test/integration/highlight_editor_spec.mjs
View File

@ -83,7 +83,11 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); // Here and elsewhere, we add a small delay between press and release
// to make sure that a pointerup event is triggered after
// selectionchange.
// It works with a value of 1ms, but we use 100ms to be sure.
await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
@ -128,7 +132,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
await page.waitForSelector( await page.waitForSelector(
@ -179,7 +183,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
await page.waitForSelector( await page.waitForSelector(
@ -225,7 +229,7 @@ describe("Highlight Editor", () => {
let rect = await getSpanRectFromText(page, 1, "Abstract"); let rect = await getSpanRectFromText(page, 1, "Abstract");
let x = rect.x + rect.width / 2; let x = rect.x + rect.width / 2;
let y = rect.y + rect.height / 2; let y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
await page.waitForSelector( await page.waitForSelector(
@ -248,7 +252,7 @@ describe("Highlight Editor", () => {
rect = await getSpanRectFromText(page, 14, "References"); rect = await getSpanRectFromText(page, 14, "References");
x = rect.x + rect.width / 2; x = rect.x + rect.width / 2;
y = rect.y + rect.height / 2; y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(1)}`); await page.waitForSelector(`${getEditorSelector(1)}`);
await page.waitForSelector( await page.waitForSelector(
`.page[data-page-number = "14"] svg.highlightOutline.selected` `.page[data-page-number = "14"] svg.highlightOutline.selected`
@ -314,7 +318,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
await page.waitForSelector( await page.waitForSelector(
@ -376,7 +380,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(sel); await page.waitForSelector(sel);
await page.waitForSelector( await page.waitForSelector(
@ -487,7 +491,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
await page.waitForSelector( await page.waitForSelector(
@ -535,7 +539,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(sel); await page.waitForSelector(sel);
await page.waitForSelector( await page.waitForSelector(
@ -577,7 +581,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(sel); await page.waitForSelector(sel);
await page.waitForSelector( await page.waitForSelector(
@ -871,7 +875,7 @@ describe("Highlight Editor", () => {
); );
const x = rect.x + 0.75 * rect.width; const x = rect.x + 0.75 * rect.width;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
const usedColor = await page.evaluate(() => { const usedColor = await page.evaluate(() => {
@ -981,7 +985,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForFunction(() => window.editingEvents.length > 0); await page.waitForFunction(() => window.editingEvents.length > 0);
let editingEvent = await page.evaluate(() => { let editingEvent = await page.evaluate(() => {
@ -1008,7 +1012,7 @@ describe("Highlight Editor", () => {
.withContext(`In ${browserName}`) .withContext(`In ${browserName}`)
.toBe(false); .toBe(false);
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForFunction(() => window.editingEvents.length > 0); await page.waitForFunction(() => window.editingEvents.length > 0);
await page.evaluate(() => { await page.evaluate(() => {
@ -1039,7 +1043,17 @@ describe("Highlight Editor", () => {
"tracemonkey.pdf", "tracemonkey.pdf",
".annotationEditorLayer", ".annotationEditorLayer",
null, null,
null, async page => {
await page.evaluate(async () => {
await window.PDFViewerApplication.initializedPromise;
window.PDFViewerApplication.eventBus.on(
"annotationeditoruimanager",
({ uiManager }) => {
window.uiManager = uiManager;
}
);
});
},
{ {
highlightEditorColors: "red=#AB0000", highlightEditorColors: "red=#AB0000",
supportsCaretBrowsingMode: true, supportsCaretBrowsingMode: true,
@ -1047,6 +1061,19 @@ describe("Highlight Editor", () => {
); );
}); });
afterEach(async () => {
for (const [, page] of pages) {
await page.evaluate(() => {
window.uiManager.reset();
});
// Disable editing mode.
await page.click("#editorHighlight");
await page.waitForSelector(
`.annotationEditorLayer:not(.highlightEditing)`
);
}
});
afterAll(async () => { afterAll(async () => {
await closePages(pages); await closePages(pages);
}); });
@ -1060,8 +1087,7 @@ describe("Highlight Editor", () => {
const rect = await getSpanRectFromText(page, 1, "Abstract"); const rect = await getSpanRectFromText(page, 1, "Abstract");
const x = rect.x + rect.width / 2; const x = rect.x + rect.width / 2;
const y = rect.y + rect.height / 2; const y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2 }); await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector(`${getEditorSelector(0)}`);
await page.keyboard.press("Escape"); await page.keyboard.press("Escape");
await page.waitForSelector( await page.waitForSelector(
@ -1092,5 +1118,41 @@ describe("Highlight Editor", () => {
}) })
); );
}); });
it("must check that selection is correctly highlighted on arrow down key pressed", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await page.click("#editorHighlight");
await page.waitForSelector(".annotationEditorLayer.highlightEditing");
await page.evaluate(() => {
const text =
"Dynamic languages such as JavaScript are more difficult to com-";
for (const el of document.querySelectorAll(
`.page[data-page-number="${1}"] > .textLayer > span`
)) {
if (el.textContent === text) {
window.getSelection().setPosition(el.firstChild, 15);
break;
}
}
});
await page.keyboard.down("Shift");
await page.keyboard.press("ArrowDown");
await page.keyboard.up("Shift");
await page.waitForSelector(getEditorSelector(0));
const usedColor = await page.evaluate(() => {
const highlight = document.querySelector(
`.page[data-page-number = "1"] .canvasWrapper > svg.highlight`
);
return highlight.getAttribute("fill");
});
expect(usedColor).withContext(`In ${browserName}`).toEqual("#AB0000");
})
);
});
}); });
}); });

2
test/test.mjs
View File

@ -949,6 +949,8 @@ async function startBrowser({ browserName, headless, startUrl }) {
"layout.css.round.enabled": true, "layout.css.round.enabled": true,
// This allow to copy some data in the clipboard. // This allow to copy some data in the clipboard.
"dom.events.asyncClipboard.clipboardItem": true, "dom.events.asyncClipboard.clipboardItem": true,
// It's helpful to see where the caret is.
"accessibility.browsewithcaret": true,
}; };
} }