Skip to content

Remove the scrollIntoView function #20953

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.

Sign up for GitHub

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

Open
calixteman wants to merge 1 commit into
base: master
Choose a base branch
from
+23 −52
Open

Remove the scrollIntoView function #20953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 11 additions & 6 deletions web/pdf_viewer.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@

--viewer-container-height: 0;
--pdfViewer-padding-bottom: 0;
--page-margin: 1px auto -8px;
--page-border: 9px solid transparent;
--page-margin-top: 10px;
--page-outline: none;
--spreadHorizontalWrapped-margin-LR: -3.5px;
--loading-icon-delay: 400ms;
--focus-ring-color: light-dark(#0060df, #0df);
Expand All @@ -42,8 +42,7 @@

@media screen and (forced-colors: active) {
--pdfViewer-padding-bottom: 9px;
--page-margin: 8px auto -1px;
--page-border: 1px solid CanvasText;
--page-outline: 1px solid CanvasText;
--spreadHorizontalWrapped-margin-LR: 3.5px;
--focus-ring-color: CanvasText;
--new-badge-bg: AccentColor;
Expand Down Expand Up @@ -142,12 +141,18 @@
direction: ltr;
width: 816px;
height: 1056px;
margin: var(--page-margin);
margin-inline: auto;
Copy link
Collaborator

@Snuffleupagus Snuffleupagus Mar 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not have a value around 10px (via a new CSS variable), to mimic the effect of the old page-border rule?

Likely related to this is that pages in spreadMode, or when using the horizontal and wrapped scrollModes, no longer have any horizontal space between them.

margin-block: var(--page-margin-top) 0;
position: relative;
overflow: visible;
border: var(--page-border);
outline: var(--page-outline);
background-clip: content-box;
background-color: var(--page-bg-color, rgb(255 255 255));
scroll-margin-top: calc(0 - var(--page-margin-top));

&:last-child {
margin-block-end: var(--page-margin-top);
}
}

.pdfViewer .dummyPage {
Expand Down
14 changes: 12 additions & 2 deletions web/pdf_viewer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ import {
PresentationModeState,
removeNullCharacters,
SCROLLBAR_PADDING,
scrollIntoView,
ScrollMode,
SpreadMode,
TextLayerMode,
Expand Down Expand Up @@ -1458,7 +1457,18 @@ class PDFViewer {
pageSpot = { left: 0, top: 0 };
}
}
scrollIntoView(div, pageSpot);

div.scrollIntoView({
behavior: "auto",

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

block: "start",
inline: pageSpot ? "start" : "nearest",
});
if (pageSpot?.top) {
this.container.scrollTop += pageSpot.top;
}
if (pageSpot?.left) {
this.container.scrollLeft += pageSpot.left;
}

// Ensure that the correct *initial* document position is set, when any
// OpenParameters are used, for documents with non-default Scroll/Spread
Expand Down
44 changes: 0 additions & 44 deletions web/ui_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,49 +69,6 @@ const CursorTool = {
// Used by `PDFViewerApplication`, and by the API unit-tests.
const AutoPrintRegExp = /\bprint\s*\(/;

/**
* Scrolls specified element into view of its parent.
* @param {HTMLElement} element - The element to be visible.
* @param {Object} [spot] - An object with optional top and left properties,
* specifying the offset from the top left edge.
* @param {number} [spot.left]
* @param {number} [spot.top]
*/
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.
let parent = element.offsetParent;
if (!parent) {
console.error("offsetParent is not set -- cannot scroll");
return;
}
let offsetY = element.offsetTop + element.clientTop;
let offsetX = element.offsetLeft + element.clientLeft;
while (
parent.clientHeight === parent.scrollHeight &&
parent.clientWidth === parent.scrollWidth
) {
offsetY += parent.offsetTop;
offsetX += parent.offsetLeft;

parent = parent.offsetParent;
if (!parent) {
return; // no need to scroll
}
}
if (spot) {
if (spot.top !== undefined) {
offsetY += spot.top;
}
if (spot.left !== undefined) {
offsetX += spot.left;
parent.scrollLeft = offsetX;
}
}
parent.scrollTop = offsetY;
}

/**
* Helper function to start monitoring the scroll event and converting them into
* PDF.js friendly one: with scroll debounce and scroll direction.
Expand Down Expand Up @@ -890,7 +847,6 @@ export {
ProgressBar,
removeNullCharacters,
SCROLLBAR_PADDING,
scrollIntoView,
ScrollMode,
SidebarView,
SpreadMode,
Expand Down
Loading