1671331 - turn off apz.force_disable_desktop_zooming_scrollbars on android

Status: RESOLVED FIXED

(Core :: Panning and Zooming, task)

Comment 1

[Timothy Nikkel (:tnikkel)]

This is green on try (I made sure of that before turning it on desktop and it continues to be green now), so we could turn this on at the beginning of the next cycle. We should just give a heads up to the fenix folks.

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1671331. Enable new scrollbar code for android too. r?kats

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae98627c9afe
Enable new scrollbar code for android too. r=kats

Comment 4

[Alexandru Michis [:malexandru]]

Backed out changeset ae98627c9afe (Bug 1671331) for causing frequent mochitest plain failures in test_bug677878.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/4859541000e7b3efb51940fd273e242e4caa9ded

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=mochitest%2Cplain&fromchange=e268681dd7b3ee3ea415f94e07f7c5b864fc1f52&tochange=d922e9a28e68f4c19a731b20e0c9bf0f191e0150&selectedTaskRun=fTZjaRGiTNaW8UsjP-YGrw.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=319293873&repo=autoland&lineNumber=4694

Comment 5

[Timothy Nikkel (:tnikkel)]

If I disable zoom to focused input for that dir the problem goes away. I'm tired of this same problem keeping coming up so I'm digging in a little bit more this time.

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1671331. Skip gfx/layers/apz/test/mochitest/helper_bug1669625.html on android. r?botond,kats

I spent a lot of effort trying to get this test to work when I originally landed it, as document in the comments on https://phabricator.services.mozilla.com/D92891

At the end of that I mistakenly concluded it magically started working because I forgot to set a pref when doing a try push.

The test is checking that a paint doesn't happen. So you must go to great care to make sure a random paint doesn't happen. The test is stable enough on desktop but on mobile some other paints creep in. I tried to fix that but I had already spent a ton of time on it and it didn't seem fruitful. Having the test on all desktop platforms should be good enough.

Flipping apz.force_disable_desktop_zooming_scrollbars pref on android exposes this failure. (The test is testing a regression from flipping that pref on desktop).

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1671331. Create a new test-only pref that disables the zoom part of ZoomToFocusedInput and use it in layout/base/tests/ mochitests. r?kats,botond

layout/base/tests/test_bug332655-1.html calls focus on an element, this triggers a call to zoomToFocusedInput. This can zoom the parent document of the test, the mochitest harness.

Then test layout/base/tests/test_bug667512.html runs. It ends by synthesizing a left arrow key and then a right arrow key. With the old scrollbar code this results in no scrolling because there is no layout scrolling that can be done either in the test or the parent document. With the new scrollbar code we can scroll the visual viewport using the keyboard so this scrolls the visual viewport of the test harness (because it got zoomed from a previous test).

The test harness being scrolled means that the synthesized click in the next test test_bug677878 can not hit its intended target.

Using the pref apz.zoom-to-focused-input.enabled to disable zoom to focused input in this directory does work to fix this, but I'm tired of doing that.

I think the proper fix here would be to disable scrolling/zooming of the test harness document. Since it's the root content document and it is not scrollable if not zoomed this is equivalent to disabling the zooming part of zoom to focused input (except for tests that open their own window). To test the zooming part of zoom to focused input we can use a test that opens it's own window: we have gfx/layers/apz/test/mochitest/test_group_zoomToFocusedInput.html for this.

We already have the pref formhelper.autozoom would seems like it would work for this purpose, except it is entangled with java stuff in mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntimeSettings.java that I don't really understand that means setting the pref in mochitest.ini doesn't seem to take affect.

So I created a new pref we can use specifically for this purpose. Hopefully we can use it instead of fully disabling the zoom to focused input, so we more closely test what we ship.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/767e71a480eb
Skip gfx/layers/apz/test/mochitest/helper_bug1669625.html on android. r=kats

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b8874ab54c65
Create a new test-only pref that disables the zoom part of ZoomToFocusedInput and use it in layout/base/tests/ mochitests.  r=botond

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f69567e1cb1
Enable new scrollbar code for android too. r=kats

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/767e71a480eb
https://hg.mozilla.org/mozilla-central/rev/b8874ab54c65
https://hg.mozilla.org/mozilla-central/rev/4f69567e1cb1