Closed Bug 1707389 Opened 3 years ago Closed 3 years ago

Double-tap to zoom doesn’t work on view-source page

Categories

(Core :: Panning and Zooming, defect)

All
macOS
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox89 --- verified
firefox90 --- verified

People

(Reporter: emilghitta, Assigned: tnikkel)

References

Details

(Whiteboard: [proton-uplift])

Attachments

(3 files)

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

  1. Launch Firefox.
  2. Access any webpage.
  3. Open the page context menu.
  4. Select the “View Page Source” option.
  5. 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).
Has Regression Range: --- → irrelevant
Has STR: --- → yes

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.

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.

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.

Attached video CleanShot 2021-04-26 at 11.43.26.mp4 (deleted) —

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.

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).

Oups, my bad 🤦‍♂️ I do have this pref set. Now that it's set to false I can reproduce the scroll-to-top issue.

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.

Assignee: nobody → tnikkel
Status: NEW → ASSIGNED
Attached file Bug 1707389. Add test. r?botond (deleted) —
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
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Whiteboard: [proton-uplift]

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:
Attachment #9218307 - Flags: approval-mozilla-beta?
Attachment #9218924 - Flags: approval-mozilla-beta?

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.

Attachment #9218307 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9218924 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: