GeckoView::ScrollBy and ScrollTo are broken
Categories
(GeckoView :: General, defect, P1)
Tracking
(firefox73 fixed)
Tracking | Status | |
---|---|---|
firefox73 | --- | fixed |
People
(Reporter: hiro, Assigned: hiro)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression)
Attachments
(3 files)
When I was auditing our usage of window.inner{Width,Height} for bug 1598487, I noticed we are using the metrics in GeckoViewContentChild.toPixels, apparently it's supposed to be the visual viewport instead of the layout viewport.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7eb6e1d050a208c759710974c7df85f0244a156a
Comment 1•5 years ago
|
||
Good catch. Sorry I missed this on my test runs. I didn't adequately exercise the GeckoView tests, clearly. I hope the fix is straightforward.
Assignee | ||
Comment 2•5 years ago
|
||
No problem at all. It's actually hard to notice. The fix is absolutely straightforward, but writing automated tests is hard, the test I added here failed on try intermittently.. :/
Assignee | ||
Comment 3•5 years ago
|
||
The tests I am going to add are really flaky on tries, I figured out the reason. The tests use double-tap-zoom to make visual viewport mismatch with layout viewport. In the failure case, the double-tap-zoom was not processed because the test started running before mZoomConstraints.mAllowDoubleTapZoom is updated in AsyncPanZoomController::UpdateZoomConstraints. I couldn't find good ways to make sure the mAllowDoubleTapZoom is updated in JUnit tests, so I am going to add one sec wait before running the test.
Assignee | ||
Comment 4•5 years ago
|
||
Since bug 1514429 window.inner{Width,Height} don't return the visual viewport
size so once after the content scale changed, i.e. the visual viewport size
doesn't match window inner size, GeckoView::ScrollBy and ScrollTo don't work
as expected. This commit has JUnit tests to generate the situation by using
double-tap-zoom on a small element in the initial visual viewport.
Assignee | ||
Comment 5•5 years ago
|
||
Depends on D56321
Assignee | ||
Comment 6•5 years ago
|
||
Depends on D56322
Updated•5 years ago
|
Comment 8•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/46e48ee15dd0
https://hg.mozilla.org/mozilla-central/rev/4cf75b510765
https://hg.mozilla.org/mozilla-central/rev/65d449af6342
Updated•5 years ago
|
Description
•