Double-tap to zoom doesn’t work on view-source page
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
People
(Reporter: emilghitta, Assigned: tnikkel)
References
Details
(Whiteboard: [proton-uplift])
Attachments
(3 files)
(deleted),
video/mp4
|
Details | |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
Affected versions
- Firefox 90.0a1 (BuildId:20210424090050)
- Firefox 89.0b3 (BuildId:20210422190146)
Affected platforms
- macOS 10.13.6
- macOS 11.3
Unaffected platforms
- Windows 10 64bit.
- Ubuntu 20.04 64bit.
Steps to reproduce
- Launch Firefox.
- Access any webpage.
- Open the page context menu.
- Select the “View Page Source” option.
- Double tap to zoom inside the page.
Expected result
- Double-tap to zoom functionality works as in Chrome. (Safari seems to have a different “view page source” functionality [embedded inside the Developer tools]).
Actual result
- The double-tap to zoom functionality doesn’t work and if the user scrolls down and double taps to zoom the page automatically scrolls back to the top.
Regression Range
- This doesn’t seem to be a regression.
Additional Notes
- Access the following link for screencast (Mozilla account needed).
Reporter | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
Our view source implementation seems to consist entirely of inline spans, and we ignore inline elements for computing double tap zoom rect.
Chrome and Safari seem to use "just zoom in 3x around the mouse cursor" a lot. In this case Safari doesn't have an analog, and Chrome seems to zoom in much less than 3x. It's zoom in seems kind of useless actually, but at least it provides user feedback. So maybe if we can't detect a useful zoom in rect based on the page content we zoom in 1.5x or something.
Comment 2•3 years ago
|
||
I question the value of allowing double-tap zoom in view source. The current approach is a bit confusing though. For example, when viewing the source of this bugzilla page and using double-tap zoom, the user will be zoomed out to a point where text becomes unreadable.
I recommend disabling the feature on view source, similar to about: pages.
Assignee | ||
Comment 3•3 years ago
|
||
Yeah, there's not much value of allowing it in view source, except that view source is an html page like any other, so presumably there is web content that would hit the same issue. And for example the issue of scrolling back to the top is something that happens in web content that I noticed before this that I do intend to fix (it's a simple patch) just haven't filed the bug yet.
I can't reproduce the zooming out of the view source of this bugzilla page though. That sounds very wrong: view source doesn't seem to allow me to zoom out at all using pinch zoom, so double tap shouldn't either.
The question what parts of this we what to fix before release is a different question. I think the scrolling to the top issue should be fixed before release.
Comment 4•3 years ago
|
||
I agree with the scrolling top issue before release.
For reference, I attached a video that shows my experience when double-tapping on bugzilla page source. Note that I can also pinch-zoom way too far out.
Assignee | ||
Comment 5•3 years ago
|
||
I can reproduce that but only if I set apz.allow_zooming_out to true (it's false by default because there are known issues with it).
Comment 6•3 years ago
|
||
Oups, my bad 🤦♂️ I do have this pref set. Now that it's set to false I can reproduce the scroll-to-top issue.
Assignee | ||
Comment 7•3 years ago
|
||
The code in ZoomToRect that calculate the rect to zoom out to does not work if the page is horizontally scrollable, and our min zoom does not allow us to zoom out to fit the full page width.
For example, the targetZoom might get set to 0.1 and our min zoom is 1. The target zoom would get updated to 1, but the scroll offset of the rect wouldn't get adjusted for the new zoom.
So change how we calculate it by taking the min zoom and then calculating the scroll offset so the old visible area is centered in the new visible area.
Updated•3 years ago
|
Assignee | ||
Comment 8•3 years ago
|
||
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a412c8fafead If a page is horizontally scrollable make zooming out using double tap to zoom work. r=botond https://hg.mozilla.org/integration/autoland/rev/8e9eee59cbe7 Add test. r=botond
Comment 10•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a412c8fafead
https://hg.mozilla.org/mozilla-central/rev/8e9eee59cbe7
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 11•3 years ago
|
||
Comment on attachment 9218307 [details]
Bug 1707389. If a page is horizontally scrollable make zooming out using double tap to zoom work. r?botond
Beta/Release Uplift Approval Request
- User impact if declined: double tap zoom on pages that are horizontally scrollable will work quite badly
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): just changes the rect calculation of what to zoom to
- String changes made/needed:
Assignee | ||
Updated•3 years ago
|
Comment 12•3 years ago
|
||
Comment on attachment 9218307 [details]
Bug 1707389. If a page is horizontally scrollable make zooming out using double tap to zoom work. r?botond
Approved for 89 beta 7, thanks.
Updated•3 years ago
|
Comment 13•3 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 14•3 years ago
|
||
This issue is verified fixed using Firefox 89.0b9 (BuildId:20210506185706) and Firefox 90.0a1 (BuildId:20210506214311) on macOS 10.14.
The view source page is no longer auto scrolled to the top if double tapping to zoom.
Description
•