Closed Bug 1636277 Opened 7 months ago Closed 1 month ago

Pinch-zooming in on BMO does not create vertical viewport scrollbar

Categories

(Core :: Panning and Zooming, defect)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox75 --- disabled
firefox76 --- disabled
firefox77 --- disabled
firefox78 --- disabled
firefox83 --- fixed

People

(Reporter: botond, Assigned: tnikkel)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

STR

  1. Run latest nightly with apz.allow_zooming=true
  2. Load https://bugzilla.mozilla.org/show_bug.cgi?id=1556556
  3. Observe that at unit zoom, there are no viewport scrollbars. This is because the document is sized to the layout viewport, and at unit zoom, the visual viewport coincides with the layout viewport. (The scrollbar which does appear is for a subframe.)
  4. Pinch-zoom in, thereby causing the visual and layout viewports to diverge.
  5. Observe that a horizontal viewport scrollbar appears.

Expected results

A vertical viewport scrollbar appears as well. (Since the visual and layout viewports are aspect-ratio locked, it follows that if the visual viewport becomes smaller than the layout viewport in the horizontal dimension, it must do so in the vertical dimension as well.)

Actual results

A vertical viewport scrollbar does not appear.

Severity: -- → S3

This likely has to do with the root element on BMO being overflow-y: hidden.

Note that overflow: hidden does not suppress creation of viewport scrollbars when zoomed on mobile, but perhaps the relevant logic is currently mobile-specific, or specific to overlay scrollbars.

(In reply to Botond Ballo [:botond] from comment #1)

This likely has to do with the root element on BMO being overflow-y: hidden.

Note that overflow: hidden does not suppress creation of viewport scrollbars when zoomed on mobile, but perhaps the relevant logic is currently mobile-specific, or specific to overlay scrollbars.

Doesn't seem to be different for overlay vs not.

If I make these Auto when the style is overflow hidden and it's the root scroll frame things seem to work as we want.

https://searchfox.org/mozilla-central/rev/61fceb7c0729773f544a9656f474e36cd636e5ea/layout/generic/nsGfxScrollFrame.cpp#314

We can't pinch zoom all root scroll frames but applying this to all root scroll frames seem reasonable.

Assignee: nobody → tnikkel
Status: NEW → ASSIGNED
Attachment #9150634 - Attachment is obsolete: true

Doing some testing on macOS. Chrome never seems to add scrollbars after pinchzooming that weren't there before, and the behaviour is the same whether I have "Show scroll bars" in system settings set to "Automatically based on mouse or trackpad" or "Always".

Safari seems to have the best behaviour, even with show scrollbars is set to always, it will add scrollbars as expected and they get added after you lift your fingers from the pinch gesture.

Doing a quick test on Windows pinchzooming google.ca, Chrome is the same, never adds scrollbars after pinchzooming that weren't there before. Edge adds non-floating scrollbars while you are pinch zooming.

It looks like neither fennec nor fenix show this scrollbar.

Making the scrollbar show is not so hard. Once it's showing there are many bugs. The worst and seems like the best place to start is: the scroll thumb is offset, it looks the right size, and moves properly, but when scrolled all the way to the top it's halfway down the track, and when scrolled to the bottom it can be partway or completely off screen. I've verified that everything on the layout side seems correct, the frame tree looks correct, the display list looks correct, the layer tree looks correct. So there is something changing the scrollbars on the compositor side?

(In reply to Timothy Nikkel (:tnikkel) from comment #6)

So there is something changing the scrollbars on the compositor side?

APZ can apply a transform to scroll thumbs to account for async scrolling (WR call site, non-WR call site), it could be that.

This comment
https://searchfox.org/mozilla-central/rev/7dafc35406b9c945189c617d427f5458933fd3fb/gfx/layers/apz/src/APZCTreeManager.cpp#3746
// |asyncTransform| represents the amount by which we have scrolled and
// zoomed since the last paint. Because the scrollbar was sized and positioned
// based on the painted content, we need to adjust it based on asyncTransform
// so that it reflects what the user is actually seeing now.
probably explains it (I'll look into the code next to confirm). I've had to change the scrollbars so that they reflect the visual viewport offset and the visual viewport size, thus likely already taking into account some (all?) of the transforms that function does, and the asyncTransform is persistant in the overflow hidden case (the painted content doesn't "sync up" with the async transform ever).

Modified ComputeTransformForScrollThumb so that the thumbs usual draws correctly, but it quick frequently flashes in the wrong position. Not sure what is going on with that yet. There is another issue which is that we can get stuck scrolled down by a small but noticeable amount even when fully zoomed out. After some debugging this seems to be caused by bug 1611660, when we add/remove the scrollbars (the kind that takes space, which is what I happened to have left my system in when testing various combinations) we send new metrics with new composition bounds, new scrollable rect, and new layout viewport. We accept the first two but ignore the layout viewport update. Then some pinch zooming causes some scroll offset changes and that makes us call KeepLayoutViewportEnclosingVisualViewport and we don't hit the early return, and then the layout viewport gets moved, and we end up scrolling back on the main thread (and not just the visual viewport offset, the scroll frame position).

Fix one thing, either expose or create two problems.

Is bugzilla the only website that has this issue?

And why do the scrollbars look different? Try this: start from default zoom (reflow zoom) and go to menu > zoom in (Ctrl++) a bunch and notice after a zooming in enough, the vertical scrollbar changes in appearance. The arrows at the top and bottom go from white to grey and the thumb becomes significantly brighter. Strange

(In reply to Will from comment #11)

Is bugzilla the only website that has this issue?

I think it may affect any site where the page is not scrollable at unit zoom.

And why do the scrollbars look different? Try this: start from default zoom (reflow zoom) and go to menu > zoom in (Ctrl++) a bunch and notice after a zooming in enough, the vertical scrollbar changes in appearance. The arrows at the top and bottom go from white to grey and the thumb becomes significantly brighter. Strange

If this happens without performing pinch-zoom at all, it's definitely worth filing a separate bug about.

(In reply to Botond Ballo [:botond] from comment #12)

(In reply to Will from comment #11)

Is bugzilla the only website that has this issue?

I think it may affect any site where the page is not scrollable at unit zoom.

And why do the scrollbars look different? Try this: start from default zoom (reflow zoom) and go to menu > zoom in (Ctrl++) a bunch and notice after a zooming in enough, the vertical scrollbar changes in appearance. The arrows at the top and bottom go from white to grey and the thumb becomes significantly brighter. Strange

If this happens without performing pinch-zoom at all, it's definitely worth filing a separate bug about.

Are you able to repro the vertical scrollbar changing in appearance after you zoom in a bunch via reflow zoom? No pinch zoom is applied at all

I am, yes. Seems like a potential bug in the scrollbar-color implementation where, past a certain zoom level, we start ignoring the custom style.

(In reply to Will from comment #11)

Is bugzilla the only website that has this issue?

Any site that uses overflow: hidden would be affected. overflow: scroll and overflow: auto should show the scrollbar on pinch zoom even if there wasn't a scrollbar visible before pinch zoom (interacting with the scrollbar may not work properly, which is known and tracked by other bugs).

Verified fixed with tnikkel's scrollbar changes in 1657822

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.