Skip to content

Commit

Permalink
Fix autolinking with highlighted search results
Browse files Browse the repository at this point in the history
The current logic assumes that all spans in the text layer contain
only one text node, and thus that the position information
returned by `highlighter._convertMatches` can be directly used
on the element's only child.

This is not true in case of highlighted search results: they will be
injected in the DOM as `<span>` elements, causing the `<span>`s
in the text layer to have more than one child.

This patch fixes the problem by properly converting the (span, offset)
pair in a (textNode, offset) pair that points to the right text node.
  • Loading branch information
nicolo-ribaudo committed Mar 2, 2025
1 parent 50c573d commit b47d815
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 4 deletions.
85 changes: 83 additions & 2 deletions test/integration/autolinker_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,46 @@
* limitations under the License.
*/

import { closePages, createPromise, loadAndWait } from "./test_utils.mjs";
import {
awaitPromise,
closePages,
createPromise,
loadAndWait,
} from "./test_utils.mjs";

function waitForLinkAnnotations(page) {
function waitForLinkAnnotations(page, pageNumber) {
return page.evaluateHandle(
number => [
new Promise(resolve => {
const { eventBus } = window.PDFViewerApplication;
eventBus.on("linkannotationsadded", function listener(e) {
if (number === undefined || e.pageNumber === number) {
resolve();
eventBus.off("linkannotationsadded", listener);
}
});
}),
],
pageNumber
);
}

function recordInitialLinkAnnotationsEvent(eventBus) {
globalThis.initialLinkAnnotationsEventFired = false;
eventBus.on(
"linkannotationsadded",
() => {
globalThis.initialLinkAnnotationsEventFired = true;
},
{ once: true }
);
}
function waitForInitialLinkAnnotations(page) {
return createPromise(page, resolve => {
if (globalThis.initialLinkAnnotationsEventFired) {
resolve();
return;
}
window.PDFViewerApplication.eventBus.on("linkannotationsadded", resolve, {
once: true,
});
Expand Down Expand Up @@ -178,4 +214,49 @@ describe("autolinker", function () {
);
});
});

describe("when highlighting search results", function () {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"issue3115r.pdf",
".annotationLayer",
null,
{ eventBusSetup: recordInitialLinkAnnotationsEvent },
{ enableAutoLinking: true }
);
});

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

it("must find links that overlap with search results", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await awaitPromise(await waitForInitialLinkAnnotations(page));

const linkAnnotationsPromise = await waitForLinkAnnotations(page, 36);

// Search for "rich.edu"
await page.click("#viewFindButton");
await page.waitForSelector("#viewFindButton", { hidden: false });
await page.type("#findInput", "rich.edu");
await page.waitForSelector(".textLayer .highlight");

await awaitPromise(linkAnnotationsPromise);

const urls = await page.$$eval(
".page[data-page-number='36'] > .annotationLayer > .linkAnnotation > a",
annotations => annotations.map(a => a.href)
);

expect(urls)
.withContext(`In ${browserName}`)
.toContain(jasmine.stringContaining("rich.edu"));
})
);
});
});
});
46 changes: 44 additions & 2 deletions web/autolinker.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,55 @@ function calculateLinkPosition(range, pdfPageView) {
return { quadPoints, rect };
}

/**
* Given a DOM node `container` and an index into its text contents `offset`,
* returns a pair consisting of text node that the `offset` actually points
* to, together with the offset relative to that text node.
* When the offset points at the boundary between two node, the result will
* point to the first text node in depth-first traversal order.
*
* For example, given this DOM:
* <p>abc<span>def</span>ghi</p>
*
* textPosition(p, 0) -> [#text "abc", 0] (before `a`)
* textPosition(p, 2) -> [#text "abc", 2] (between `b` and `c`)
* textPosition(p, 3) -> [#text "abc", 3] (after `c`)
* textPosition(p, 5) -> [#text "def", 2] (between `e` and `f`)
* textPosition(p, 6) -> [#text "def", 3] (after `f`)
*/
function textPosition(container, offset) {
let currentContainer = container;
do {
if (currentContainer.nodeType === Node.TEXT_NODE) {
const currentLength = currentContainer.textContent.length;
if (offset <= currentLength) {
return [currentContainer, offset];
}
offset -= currentLength;
} else if (currentContainer.firstChild) {
currentContainer = currentContainer.firstChild;
continue;
}

while (!currentContainer.nextSibling && currentContainer !== container) {
currentContainer = currentContainer.parentNode;
}
if (currentContainer !== container) {
currentContainer = currentContainer.nextSibling;
}
} while (currentContainer !== container);
throw new Error("Offset is bigger than container's contents length.");
}

function createLinkAnnotation({ url, index, length }, pdfPageView, id) {
const highlighter = pdfPageView._textHighlighter;
const [{ begin, end }] = highlighter._convertMatches([index], [length]);

const range = new Range();
range.setStart(highlighter.textDivs[begin.divIdx].firstChild, begin.offset);
range.setEnd(highlighter.textDivs[end.divIdx].firstChild, end.offset);
range.setStart(
...textPosition(highlighter.textDivs[begin.divIdx], begin.offset)
);
range.setEnd(...textPosition(highlighter.textDivs[end.divIdx], end.offset));

return {
id: `inferred_link_${id}`,
Expand Down

0 comments on commit b47d815

Please sign in to comment.