Skip to content

Commit 2d25437

Browse files
calixtemancalixteman
authored
Merge pull request #18424 from calixteman/avoid_hovering_when_selecting
[Editor] Disable existing highlights when drawing a new one (bug 1879035)
2 parents df5bc54 + 4e7c30d commit 2d25437

File tree

4 files changed

+172
-14
lines changed

4 files changed

+172
-14
lines changed

‎src/display/editor/annotation_editor_layer.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,10 @@ class AnnotationEditorLayer {
230230
this.#uiManager.addCommands(params);
231231
}
232232

233+
toggleDrawing(enabled = false) {
234+
this.div.classList.toggle("drawing", !enabled);
235+
}
236+
233237
togglePointerEvents(enabled = false) {
234238
this.div.classList.toggle("disabled", !enabled);
235239
}
@@ -388,7 +392,12 @@ class AnnotationEditorLayer {
388392
// Unselect all the editors in order to let the user select some text
389393
// without being annoyed by an editor toolbar.
390394
this.#uiManager.unselectAll();
391-
if (event.target === this.#textLayer.div) {
395+
const { target } = event;
396+
if (
397+
target === this.#textLayer.div ||
398+
(target.classList.contains("endOfContent") &&
399+
this.#textLayer.div.contains(target))
400+
) {
392401
const { isMac } = FeatureTest.platform;
393402
if (event.button !== 0 || (event.ctrlKey && isMac)) {
394403
// Do nothing on right click.
@@ -400,6 +409,7 @@ class AnnotationEditorLayer {
400409
/* updateButton = */ true
401410
);
402411
this.#textLayer.div.classList.add("free");
412+
this.toggleDrawing();
403413
HighlightEditor.startHighlighting(
404414
this,
405415
this.#uiManager.direction === "ltr",
@@ -409,6 +419,7 @@ class AnnotationEditorLayer {
409419
"pointerup",
410420
() => {
411421
this.#textLayer.div.classList.remove("free");
422+
this.toggleDrawing(true);
412423
},
413424
{ once: true, signal: this.#uiManager._signal }
414425
);

‎src/display/editor/tools.js

+32-13
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,19 @@ class AnnotationEditorUIManager {
967967
: anchorNode;
968968
}
969969

970+
#getLayerForTextLayer(textLayer) {
971+
const { currentLayer } = this;
972+
if (currentLayer.hasTextLayer(textLayer)) {
973+
return currentLayer;
974+
}
975+
for (const layer of this.#allLayers.values()) {
976+
if (layer.hasTextLayer(textLayer)) {
977+
return layer;
978+
}
979+
}
980+
return null;
981+
}
982+
970983
highlightSelection(methodOfCreation = "") {
971984
const selection = document.getSelection();
972985
if (!selection || selection.isCollapsed) {
@@ -988,19 +1001,17 @@ class AnnotationEditorUIManager {
9881001
});
9891002
this.showAllEditors("highlight", true, /* updateButton = */ true);
9901003
}
991-
for (const layer of this.#allLayers.values()) {
992-
if (layer.hasTextLayer(textLayer)) {
993-
layer.createAndAddNewEditor({ x: 0, y: 0 }, false, {
994-
methodOfCreation,
995-
boxes,
996-
anchorNode,
997-
anchorOffset,
998-
focusNode,
999-
focusOffset,
1000-
text,
1001-
});
1002-
break;
1003-
}
1004+
const layer = this.#getLayerForTextLayer(textLayer);
1005+
if (layer) {
1006+
layer.createAndAddNewEditor({ x: 0, y: 0 }, false, {
1007+
methodOfCreation,
1008+
boxes,
1009+
anchorNode,
1010+
anchorOffset,
1011+
focusNode,
1012+
focusOffset,
1013+
text,
1014+
});
10041015
}
10051016
}
10061017

@@ -1062,6 +1073,7 @@ class AnnotationEditorUIManager {
10621073
}
10631074
return;
10641075
}
1076+
10651077
this.#highlightToolbar?.hide();
10661078
this.#selectedTextNode = anchorNode;
10671079
this.#dispatchUpdateStates({
@@ -1081,12 +1093,19 @@ class AnnotationEditorUIManager {
10811093

10821094
this.#highlightWhenShiftUp = this.isShiftKeyDown;
10831095
if (!this.isShiftKeyDown) {
1096+
const activeLayer =
1097+
this.#mode === AnnotationEditorType.HIGHLIGHT
1098+
? this.#getLayerForTextLayer(textLayer)
1099+
: null;
1100+
activeLayer?.toggleDrawing();
1101+
10841102
const signal = this._signal;
10851103
const pointerup = e => {
10861104
if (e.type === "pointerup" && e.button !== 0) {
10871105
// Do nothing on right click.
10881106
return;
10891107
}
1108+
activeLayer?.toggleDrawing(true);
10901109
window.removeEventListener("pointerup", pointerup);
10911110
window.removeEventListener("blur", pointerup);
10921111
if (e.type === "pointerup") {

‎test/integration/highlight_editor_spec.mjs

+124
Original file line numberDiff line numberDiff line change
@@ -1766,4 +1766,128 @@ describe("Highlight Editor", () => {
17661766
);
17671767
});
17681768
});
1769+
1770+
describe("Draw a free highlight with the pointer hovering an existing highlight", () => {
1771+
let pages;
1772+
1773+
beforeAll(async () => {
1774+
pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
1775+
});
1776+
1777+
afterAll(async () => {
1778+
await closePages(pages);
1779+
});
1780+
1781+
it("must check that an existing highlight is ignored on hovering", async () => {
1782+
await Promise.all(
1783+
pages.map(async ([browserName, page]) => {
1784+
await switchToHighlight(page);
1785+
1786+
const rect = await getSpanRectFromText(page, 1, "Abstract");
1787+
const editorSelector = getEditorSelector(0);
1788+
const x = rect.x + rect.width / 2;
1789+
let y = rect.y + rect.height / 2;
1790+
await page.mouse.click(x, y, { count: 2, delay: 100 });
1791+
await page.waitForSelector(editorSelector);
1792+
await waitForSerialized(page, 1);
1793+
await page.keyboard.press("Escape");
1794+
await page.waitForSelector(`${editorSelector}:not(.selectedEditor)`);
1795+
1796+
const counterHandle = await page.evaluateHandle(sel => {
1797+
const el = document.querySelector(sel);
1798+
const counter = { count: 0 };
1799+
el.addEventListener(
1800+
"pointerover",
1801+
() => {
1802+
counter.count += 1;
1803+
},
1804+
{ capture: true }
1805+
);
1806+
return counter;
1807+
}, editorSelector);
1808+
1809+
const clickHandle = await waitForPointerUp(page);
1810+
y = rect.y - rect.height;
1811+
await page.mouse.move(x, y);
1812+
await page.mouse.down();
1813+
for (
1814+
const endY = rect.y + 2 * rect.height;
1815+
y <= endY;
1816+
y += rect.height / 10
1817+
) {
1818+
await page.mouse.move(x, y);
1819+
}
1820+
await page.mouse.up();
1821+
await awaitPromise(clickHandle);
1822+
1823+
const { count } = await counterHandle.jsonValue();
1824+
expect(count).withContext(`In ${browserName}`).toEqual(0);
1825+
})
1826+
);
1827+
});
1828+
});
1829+
1830+
describe("Select text with the pointer hovering an existing highlight", () => {
1831+
let pages;
1832+
1833+
beforeAll(async () => {
1834+
pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
1835+
});
1836+
1837+
afterAll(async () => {
1838+
await closePages(pages);
1839+
});
1840+
1841+
it("must check that an existing highlight is ignored on hovering", async () => {
1842+
await Promise.all(
1843+
pages.map(async ([browserName, page]) => {
1844+
await switchToHighlight(page);
1845+
1846+
const rect = await getSpanRectFromText(
1847+
page,
1848+
1,
1849+
"ternative compilation technique for dynamically-typed languages"
1850+
);
1851+
const editorSelector = getEditorSelector(0);
1852+
const x = rect.x + rect.width / 2;
1853+
let y = rect.y + rect.height / 2;
1854+
await page.mouse.click(x, y, { count: 3, delay: 100 });
1855+
await page.waitForSelector(editorSelector);
1856+
await waitForSerialized(page, 1);
1857+
await page.keyboard.press("Escape");
1858+
await page.waitForSelector(`${editorSelector}:not(.selectedEditor)`);
1859+
1860+
const counterHandle = await page.evaluateHandle(sel => {
1861+
const el = document.querySelector(sel);
1862+
const counter = { count: 0 };
1863+
el.addEventListener(
1864+
"pointerover",
1865+
() => {
1866+
counter.count += 1;
1867+
},
1868+
{ capture: true }
1869+
);
1870+
return counter;
1871+
}, editorSelector);
1872+
1873+
const clickHandle = await waitForPointerUp(page);
1874+
y = rect.y - 3 * rect.height;
1875+
await page.mouse.move(x, y);
1876+
await page.mouse.down();
1877+
for (
1878+
const endY = rect.y + 3 * rect.height;
1879+
y <= endY;
1880+
y += rect.height / 10
1881+
) {
1882+
await page.mouse.move(x, y);
1883+
}
1884+
await page.mouse.up();
1885+
await awaitPromise(clickHandle);
1886+
1887+
const { count } = await counterHandle.jsonValue();
1888+
expect(count).withContext(`In ${browserName}`).toEqual(0);
1889+
})
1890+
);
1891+
});
1892+
});
17691893
});

‎web/annotation_editor_layer_builder.css

+4
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,10 @@
116116
.selectedEditor {
117117
z-index: 100000 !important;
118118
}
119+
120+
&.drawing * {
121+
pointer-events: none !important;
122+
}
119123
}
120124

121125
.annotationEditorLayer.waiting {

0 commit comments

Comments
 (0)