From 243659380a7e1af0fe4f133218b11acca9085251 Mon Sep 17 00:00:00 2001 From: calixteman Date: Sun, 22 Mar 2026 16:50:06 +0100 Subject: [PATCH] Correctly scroll the search result in the viewport with rotated pdfs (bug 2021392) --- test/integration/find_spec.mjs | 52 ++++++++++++++++++++++ test/pdfs/.gitignore | 1 + test/pdfs/hello_world_rotated.pdf | 74 +++++++++++++++++++++++++++++++ web/pdf_find_controller.js | 17 ++----- web/text_highlighter.js | 15 +++---- web/text_layer_builder.css | 1 + web/ui_utils.js | 26 ++--------- 7 files changed, 141 insertions(+), 45 deletions(-) create mode 100644 test/pdfs/hello_world_rotated.pdf diff --git a/test/integration/find_spec.mjs b/test/integration/find_spec.mjs index 91ea589cc..e967be54d 100644 --- a/test/integration/find_spec.mjs +++ b/test/integration/find_spec.mjs @@ -177,4 +177,56 @@ describe("find bar", () => { ); }); }); + + describe("Check that the search results are correctly visible in rotated PDFs (bug 2021392)", () => { + let pages; + + beforeEach(async () => { + pages = await loadAndWait( + "hello_world_rotated.pdf", + ".textLayer", + "page-fit" + ); + }); + + afterEach(async () => { + await closePages(pages); + }); + + it("must scroll each match into the viewport when navigating search results", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#viewFindButton"); + await page.waitForSelector("#findInput", { visible: true }); + await page.type("#findInput", "hello"); + await page.waitForSelector("#findInput[data-status='']"); + + for (let i = 0; i < 5; i++) { + if (i > 0) { + await page.click("#findNextButton"); + await page.waitForSelector("#findInput[data-status='']"); + } + + // Verify we are on the expected match number. + const resultElement = + await page.waitForSelector("#findResultsCount"); + const resultText = await resultElement.evaluate( + el => el.textContent + ); + expect(resultText) + .withContext(`In ${browserName}, match ${i + 1}`) + .toEqual(`${FSI}${i + 1}${PDI} of ${FSI}5${PDI} matches`); + + // The selected highlight must be visible in the viewport. + const selected = await page.waitForSelector( + ".textLayer .highlight.selected" + ); + expect(await selected.isIntersectingViewport()) + .withContext(`In ${browserName}, match ${i + 1}`) + .toBeTrue(); + } + }) + ); + }); + }); }); diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore index 452344cf1..e6936d0f7 100644 --- a/test/pdfs/.gitignore +++ b/test/pdfs/.gitignore @@ -891,3 +891,4 @@ !extractPages_null_in_array.pdf !issue20930.pdf !text_rise_eol_bug.pdf +!hello_world_rotated.pdf diff --git a/test/pdfs/hello_world_rotated.pdf b/test/pdfs/hello_world_rotated.pdf new file mode 100644 index 000000000..c70d16031 --- /dev/null +++ b/test/pdfs/hello_world_rotated.pdf @@ -0,0 +1,74 @@ +%PDF-1.4 +%âãÏÓ +8 0 obj +<< /Length 43 >> +stream +BT /F1 36 Tf 210 370 Td (Hello world) Tj ET +endstream +endobj +9 0 obj +<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica-Bold >> +endobj +3 0 obj +<< /Type /Page /Parent 2 0 R + /MediaBox [0 0 612 792] + /Rotate 90 + /Contents 8 0 R + /Resources << /Font << /F1 9 0 R >> >> +>> +endobj +4 0 obj +<< /Type /Page /Parent 2 0 R + /MediaBox [0 0 612 792] + /Rotate 90 + /Contents 8 0 R + /Resources << /Font << /F1 9 0 R >> >> +>> +endobj +5 0 obj +<< /Type /Page /Parent 2 0 R + /MediaBox [0 0 612 792] + /Rotate 90 + /Contents 8 0 R + /Resources << /Font << /F1 9 0 R >> >> +>> +endobj +6 0 obj +<< /Type /Page /Parent 2 0 R + /MediaBox [0 0 612 792] + /Rotate 90 + /Contents 8 0 R + /Resources << /Font << /F1 9 0 R >> >> +>> +endobj +7 0 obj +<< /Type /Page /Parent 2 0 R + /MediaBox [0 0 612 792] + /Rotate 90 + /Contents 8 0 R + /Resources << /Font << /F1 9 0 R >> >> +>> +endobj +2 0 obj +<< /Type /Pages /Kids [3 0 R 4 0 R 5 0 R 6 0 R 7 0 R] /Count 5 >> +endobj +1 0 obj +<< /Type /Catalog /Pages 2 0 R >> +endobj +xref +0 10 +0000000000 65535 f +0000001009 00000 n +0000000928 00000 n +0000000183 00000 n +0000000332 00000 n +0000000481 00000 n +0000000630 00000 n +0000000779 00000 n +0000000015 00000 n +0000000108 00000 n +trailer +<< /Size 10 /Root 1 0 R >> +startxref +1058 +%%EOF diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index c1b772db5..c1b8f1f31 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -17,8 +17,8 @@ /** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./pdf_link_service.js").PDFLinkService} PDFLinkService */ -import { binarySearchFirstItem, scrollIntoView } from "./ui_utils.js"; import { getCharacterType, getNormalizeWithNFKC } from "./pdf_find_utils.js"; +import { binarySearchFirstItem } from "./ui_utils.js"; const FindState = { FOUND: 0, @@ -28,7 +28,6 @@ const FindState = { }; const FIND_TIMEOUT = 250; // ms -const MATCH_SCROLL_OFFSET_TOP = -50; // px const CHARACTERS_TO_NORMALIZE = { "\u2010": "-", // Hyphen @@ -553,7 +552,6 @@ class PDFFindController { /** * @typedef {Object} PDFFindControllerScrollMatchIntoViewParams * @property {HTMLElement} element - * @property {number} selectedLeft * @property {number} pageIndex * @property {number} matchIndex */ @@ -562,12 +560,7 @@ class PDFFindController { * Scroll the current match into view. * @param {PDFFindControllerScrollMatchIntoViewParams} */ - scrollMatchIntoView({ - element = null, - selectedLeft = 0, - pageIndex = -1, - matchIndex = -1, - }) { + scrollMatchIntoView({ element = null, pageIndex = -1, matchIndex = -1 }) { if (!this._scrollMatches || !element) { return; } else if (matchIndex === -1 || matchIndex !== this._selected.matchIdx) { @@ -576,11 +569,7 @@ class PDFFindController { return; } this._scrollMatches = false; // Ensure that scrolling only happens once. - const spot = { - top: MATCH_SCROLL_OFFSET_TOP, - left: selectedLeft, - }; - scrollIntoView(element, spot, /* scrollMatches = */ true); + element.scrollIntoView({ block: "start", inline: "center" }); } #reset() { diff --git a/web/text_highlighter.js b/web/text_highlighter.js index 72d883f46..90a9e898f 100644 --- a/web/text_highlighter.js +++ b/web/text_highlighter.js @@ -195,11 +195,9 @@ class TextHighlighter { div.append(span); if (className.includes("selected")) { - const { left } = span.getClientRects()[0]; - const parentLeft = div.getBoundingClientRect().left; - return left - parentLeft; + return span; } - return 0; + return null; } div.append(node); @@ -233,7 +231,7 @@ class TextHighlighter { const end = match.end; const isSelected = isSelectedPage && i === selectedMatchIdx; const highlightSuffix = isSelected ? " selected" : ""; - let selectedLeft = 0; + let selectedSpan = null; // Match inside new div. if (!prevEnd || begin.divIdx !== prevEnd.divIdx) { @@ -248,14 +246,14 @@ class TextHighlighter { } if (begin.divIdx === end.divIdx) { - selectedLeft = appendTextToDiv( + selectedSpan = appendTextToDiv( begin.divIdx, begin.offset, end.offset, "highlight" + highlightSuffix ); } else { - selectedLeft = appendTextToDiv( + selectedSpan = appendTextToDiv( begin.divIdx, begin.offset, infinity.offset, @@ -271,8 +269,7 @@ class TextHighlighter { if (isSelected) { // Attempt to scroll the selected match into view. findController.scrollMatchIntoView({ - element: textDivs[begin.divIdx], - selectedLeft, + element: selectedSpan, pageIndex: pageIdx, matchIndex: selectedMatchIdx, }); diff --git a/web/text_layer_builder.css b/web/text_layer_builder.css index 8dbf97bce..98f48921b 100644 --- a/web/text_layer_builder.css +++ b/web/text_layer_builder.css @@ -110,6 +110,7 @@ &.selected { background-color: var(--highlight-selected-bg-color); backdrop-filter: var(--highlight-selected-backdrop-filter); + scroll-margin-top: 50px; } } diff --git a/web/ui_utils.js b/web/ui_utils.js index 90ab0c893..e72400ac9 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -13,8 +13,6 @@ * limitations under the License. */ -import { MathClamp } from "pdfjs-lib"; - const DEFAULT_SCALE_VALUE = "auto"; const DEFAULT_SCALE = 1.0; const DEFAULT_SCALE_DELTA = 1.1; @@ -78,11 +76,8 @@ const AutoPrintRegExp = /\bprint\s*\(/; * specifying the offset from the top left edge. * @param {number} [spot.left] * @param {number} [spot.top] - * @param {boolean} [scrollMatches] - When scrolling search results into view, - * ignore elements that either: Contains marked content identifiers, - * or have the CSS-rule `overflow: hidden;` set. The default value is `false`. */ -function scrollIntoView(element, spot, scrollMatches = false) { +function scrollIntoView(element, spot) { // Assuming offsetParent is available (it's not available when viewer is in // hidden iframe or object). We have to scroll: if the offsetParent is not set // producing the error. See also animationStarted. @@ -94,11 +89,8 @@ function scrollIntoView(element, spot, scrollMatches = false) { let offsetY = element.offsetTop + element.clientTop; let offsetX = element.offsetLeft + element.clientLeft; while ( - (parent.clientHeight === parent.scrollHeight && - parent.clientWidth === parent.scrollWidth) || - (scrollMatches && - (parent.classList.contains("markedContent") || - getComputedStyle(parent).overflow === "hidden")) + parent.clientHeight === parent.scrollHeight && + parent.clientWidth === parent.scrollWidth ) { offsetY += parent.offsetTop; offsetX += parent.offsetLeft; @@ -113,17 +105,7 @@ function scrollIntoView(element, spot, scrollMatches = false) { offsetY += spot.top; } if (spot.left !== undefined) { - if (scrollMatches) { - const elementWidth = element.getBoundingClientRect().width; - const padding = MathClamp( - (parent.clientWidth - elementWidth) / 2, - 20, - 400 - ); - offsetX += spot.left - padding; - } else { - offsetX += spot.left; - } + offsetX += spot.left; parent.scrollLeft = offsetX; } }