double tap on reddit with always show scrollbars show one frame at wrong scale
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox88 | --- | unaffected |
firefox89 | --- | fixed |
firefox90 | --- | fixed |
People
(Reporter: tnikkel, Assigned: tnikkel)
References
(Regression)
Details
(Keywords: regression, Whiteboard: [proton-uplift])
Attachments
(2 files)
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
Same str as bug 1707234. Not technically regressed by bug 1707234 but just exposed after we fix that bug because that bug prevents us from seeing this problem.
The problem is that we start the zoom in, this triggers scrollbars to be created on the content side, this sends a frame metrics update, apz sees that the composition bounds width have changed (because they are affected by these scrollbars) and thinks that means the resolution has been updated from content, so updates to that resolution, and updates the sampled state too.
I think the fix is to just send the composition bounds width not factoring in scrollbars and use it for this check. The other uses of composition bounds look fine to have scrollbars affect them.
The composition bounds width check has a todo that we should just rely on IsResolutionUpdate instead so we can remove this when that gets fixed.
https://searchfox.org/mozilla-central/rev/ff3a3fc9fb164df98d7fda4a25a72a01445993cd/gfx/layers/apz/src/AsyncPanZoomController.cpp#4991
Assignee | ||
Comment 1•3 years ago
|
||
I actually changed the composition bounds to factor in these scrollbars (scrollbars that are only visible after pinch zooming) in https://phabricator.services.mozilla.com/D85708 in which I wasn't entirely convinced of the change, but it seemed like the correct thing. I think I'm still in that state because: that change hasn't resulted in any other problems in the many months its been landed afaik, so a point in its favour. It has resulted in this bug, so a point against it, and we are back where we started.
I did a check of other composition bounds users and they are seem fine.
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
Because adding or removing a scrollbar can change without the resolution changing (in particular, when zooming in we can add scrollbars) and we don't want to detect that content has updated it's resolution and override our (apz) resolution in that case.
This happens when double tapping creates a scrollbar and the resulting repaint updates the apz resolution while the zoom animation is ongoing, messing it up for a frame.
Adding a field for frame metrics just for this is kind of gross, but it can go away when that todo is fixed in NotifyLayersUpdated.
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 3•3 years ago
|
||
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/beabe76b5f2e Look at the composition width _ignoring_ scrollbars when looking for resolution changes. r=botond https://hg.mozilla.org/integration/autoland/rev/162bc5a733da Add test. r=botond
Comment 5•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/beabe76b5f2e
https://hg.mozilla.org/mozilla-central/rev/162bc5a733da
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 6•3 years ago
|
||
Comment on attachment 9218276 [details]
Bug 1707417. Look at the composition width ignoring scrollbars when looking for resolution changes. r?botond
Beta/Release Uplift Approval Request
- User impact if declined: ugly experience double tap zooming if user has scrollbars shown always on sites that don't have scrollbars to start with
- 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): simple targeted fix
- String changes made/needed:
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 7•3 years ago
|
||
Comment on attachment 9218276 [details]
Bug 1707417. Look at the composition width ignoring scrollbars when looking for resolution changes. r?botond
Approved for 89 beta 7, thanks.
Updated•3 years ago
|
Comment 8•3 years ago
|
||
bugherder uplift |
Updated•2 years ago
|
Description
•