Skip to content

Enhance Magnifier Functionality for Cross-Mode Compatibility#5

Open
suhailshub-arch wants to merge 1 commit into
jagermesh:masterfrom
suhailshub-arch:scrolling-issue
Open

Enhance Magnifier Functionality for Cross-Mode Compatibility#5
suhailshub-arch wants to merge 1 commit into
jagermesh:masterfrom
suhailshub-arch:scrolling-issue

Conversation

@suhailshub-arch
Copy link
Copy Markdown

This pull request addresses an issue where the magnifier feature failed to synchronize correctly when scrolling across different document modes (standards mode and quirks mode).

The fix involves updating the logic for calculating scroll offsets to ensure compatibility with both modes. By prioritizing window.pageXOffset and window.pageYOffset for scroll positions which works in both standards and quirks mode, and falling back to document.documentElement.scrollLeft/document.documentElement.scrollTop where necessary (when older browsers are used), we achieve consistent behavior across all browsers and document modes.

This update ensures a seamless user experience with the magnifier feature regardless of the document rendering mode.

Note: document.documentElement.scrollLeft and document.documentElement.scrollTop only works in standards mode

…set logic for standards and quirks mode compatibility in html-magnifier.js.
Comment thread html-magnifier.js
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think the || is going to work as intended.
Consider the case when window.pageXOffset is 0; then you want it to be used (if I understand correctly), but it looks like this code will use document.documentElement.scrollLeft instead (which might not exist, depending on the platform; I'm not sure what the possibilities are).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh, I see, I guess it could still work, it's just pretty mind-bending. I withdraw my objection :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants