Skip to content

[Editor] Disable existing highlights when drawing a new one (bug 1879035) #18424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/display/editor/annotation_editor_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ class AnnotationEditorLayer {
this.#uiManager.addCommands(params);
}

toggleDrawing(enabled = false) {
this.div.classList.toggle("drawing", !enabled);
}

togglePointerEvents(enabled = false) {
this.div.classList.toggle("disabled", !enabled);
}
Expand Down Expand Up @@ -388,7 +392,12 @@ class AnnotationEditorLayer {
// Unselect all the editors in order to let the user select some text
// without being annoyed by an editor toolbar.
this.#uiManager.unselectAll();
if (event.target === this.#textLayer.div) {
const { target } = event;
if (
target === this.#textLayer.div ||
(target.classList.contains("endOfContent") &&
this.#textLayer.div.contains(target))
) {
const { isMac } = FeatureTest.platform;
if (event.button !== 0 || (event.ctrlKey && isMac)) {
// Do nothing on right click.
Expand All @@ -400,6 +409,7 @@ class AnnotationEditorLayer {
/* updateButton = */ true
);
this.#textLayer.div.classList.add("free");
this.toggleDrawing();
HighlightEditor.startHighlighting(
this,
this.#uiManager.direction === "ltr",
Expand All @@ -409,6 +419,7 @@ class AnnotationEditorLayer {
"pointerup",
() => {
this.#textLayer.div.classList.remove("free");
this.toggleDrawing(true);
},
{ once: true, signal: this.#uiManager._signal }
);
Expand Down
45 changes: 32 additions & 13 deletions src/display/editor/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,19 @@ class AnnotationEditorUIManager {
: anchorNode;
}

#getLayerForTextLayer(textLayer) {
const { currentLayer } = this;
if (currentLayer.hasTextLayer(textLayer)) {
return currentLayer;
}
for (const layer of this.#allLayers.values()) {
if (layer.hasTextLayer(textLayer)) {
return layer;
}
}
return null;
}

highlightSelection(methodOfCreation = "") {
const selection = document.getSelection();
if (!selection || selection.isCollapsed) {
Expand All @@ -988,19 +1001,17 @@ class AnnotationEditorUIManager {
});
this.showAllEditors("highlight", true, /* updateButton = */ true);
}
for (const layer of this.#allLayers.values()) {
if (layer.hasTextLayer(textLayer)) {
layer.createAndAddNewEditor({ x: 0, y: 0 }, false, {
methodOfCreation,
boxes,
anchorNode,
anchorOffset,
focusNode,
focusOffset,
text,
});
break;
}
const layer = this.#getLayerForTextLayer(textLayer);
if (layer) {
layer.createAndAddNewEditor({ x: 0, y: 0 }, false, {
methodOfCreation,
boxes,
anchorNode,
anchorOffset,
focusNode,
focusOffset,
text,
});
}
}

Expand Down Expand Up @@ -1062,6 +1073,7 @@ class AnnotationEditorUIManager {
}
return;
}

this.#highlightToolbar?.hide();
this.#selectedTextNode = anchorNode;
this.#dispatchUpdateStates({
Expand All @@ -1081,12 +1093,19 @@ class AnnotationEditorUIManager {

this.#highlightWhenShiftUp = this.isShiftKeyDown;
if (!this.isShiftKeyDown) {
const activeLayer =
this.#mode === AnnotationEditorType.HIGHLIGHT
? this.#getLayerForTextLayer(textLayer)
: null;
activeLayer?.toggleDrawing();

const signal = this._signal;
const pointerup = e => {
if (e.type === "pointerup" && e.button !== 0) {
// Do nothing on right click.
return;
}
activeLayer?.toggleDrawing(true);
window.removeEventListener("pointerup", pointerup);
window.removeEventListener("blur", pointerup);
if (e.type === "pointerup") {
Expand Down
124 changes: 124 additions & 0 deletions test/integration/highlight_editor_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1766,4 +1766,128 @@ describe("Highlight Editor", () => {
);
});
});

describe("Draw a free highlight with the pointer hovering an existing highlight", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
});

afterAll(async () => {
await closePages(pages);
});

it("must check that an existing highlight is ignored on hovering", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await switchToHighlight(page);

const rect = await getSpanRectFromText(page, 1, "Abstract");
const editorSelector = getEditorSelector(0);
const x = rect.x + rect.width / 2;
let y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 2, delay: 100 });
await page.waitForSelector(editorSelector);
await waitForSerialized(page, 1);
await page.keyboard.press("Escape");
await page.waitForSelector(`${editorSelector}:not(.selectedEditor)`);

const counterHandle = await page.evaluateHandle(sel => {
const el = document.querySelector(sel);
const counter = { count: 0 };
el.addEventListener(
"pointerover",
() => {
counter.count += 1;
},
{ capture: true }
);
return counter;
}, editorSelector);

const clickHandle = await waitForPointerUp(page);
y = rect.y - rect.height;
await page.mouse.move(x, y);
await page.mouse.down();
for (
const endY = rect.y + 2 * rect.height;
y <= endY;
y += rect.height / 10
) {
await page.mouse.move(x, y);
}
await page.mouse.up();
await awaitPromise(clickHandle);

const { count } = await counterHandle.jsonValue();
expect(count).withContext(`In ${browserName}`).toEqual(0);
})
);
});
});

describe("Select text with the pointer hovering an existing highlight", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
});

afterAll(async () => {
await closePages(pages);
});

it("must check that an existing highlight is ignored on hovering", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await switchToHighlight(page);

const rect = await getSpanRectFromText(
page,
1,
"ternative compilation technique for dynamically-typed languages"
);
const editorSelector = getEditorSelector(0);
const x = rect.x + rect.width / 2;
let y = rect.y + rect.height / 2;
await page.mouse.click(x, y, { count: 3, delay: 100 });
await page.waitForSelector(editorSelector);
await waitForSerialized(page, 1);
await page.keyboard.press("Escape");
await page.waitForSelector(`${editorSelector}:not(.selectedEditor)`);

const counterHandle = await page.evaluateHandle(sel => {
const el = document.querySelector(sel);
const counter = { count: 0 };
el.addEventListener(
"pointerover",
() => {
counter.count += 1;
},
{ capture: true }
);
return counter;
}, editorSelector);

const clickHandle = await waitForPointerUp(page);
y = rect.y - 3 * rect.height;
await page.mouse.move(x, y);
await page.mouse.down();
for (
const endY = rect.y + 3 * rect.height;
y <= endY;
y += rect.height / 10
) {
await page.mouse.move(x, y);
}
await page.mouse.up();
await awaitPromise(clickHandle);

const { count } = await counterHandle.jsonValue();
expect(count).withContext(`In ${browserName}`).toEqual(0);
})
);
});
});
});
4 changes: 4 additions & 0 deletions web/annotation_editor_layer_builder.css
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@
.selectedEditor {
z-index: 100000 !important;
}

&.drawing * {
pointer-events: none !important;
}
}

.annotationEditorLayer.waiting {
Expand Down