Closed Bug 1633322 Opened 5 years ago Closed 4 years ago

PanZoomController erroneously returns INPUT_RESULT_HANDLED_CONTENT

Categories

(GeckoView :: IME, defect, P1)

Product:
GeckoView
Component:
IME
Platform:
Unspecified
All
Type:
defect

Tracking

(firefox81 wontfix, firefox82 fixed)

Status:
RESOLVED FIXED
Milestone:
81 Branch
Tracking Flags:
Tracking Status
firefox81 --- wontfix
firefox82 --- fixed

People

(Reporter: petru, Assigned: snorp)

References

Details

(Whiteboard: [geckoview:m78][geckoview:m79][geckoview:m80][geckoview:m81])

Attachments

(4 files, 2 obsolete files)

Bug 1633322 - Combine APZEventResult::mTargetIsRoot and mHitRegionWithApzAwareListeners into a single flag. r=kats
47 bytes, text/x-phabricator-request
Details | Review
Bug 1633322 - Turn APZEventResult::mHandledByRootApzc into a Maybe. r=kats
47 bytes, text/x-phabricator-request
Details | Review
Bug 1633322 - Add an input block callback facility to allow senders of input events to receive a delayed notification of whether the event was processed by the root APZC. r=kats
47 bytes, text/x-phabricator-request
Details | Review
Bug 1633322 - Consider content handling for `onTouchEventForResult`
47 bytes, text/x-phabricator-request
Details | Review

Seen while browsing github.com in Fenix / GeckoView Example by logging the INPUT_RESULT PZC returns.
The page is too long to fit the screen but when scrolling it GV returns INPUT_RESULT_HANDLED_CONTENT signaling the webpage handled the scroll and not the browser. I think this is an error.

(InputResult log was added here)
(apz.inputqueue log was added here)

9:05:06.725 I/apz.inputqueue: Line 185: Will consume touch event
9:05:06.725 I/InputResult: INPUT_RESULT_HANDLED_CONTENT
9:05:06.787 I/apz.inputqueue: Line 182: dropping event due to block 0xadcc6380 being in slop
9:05:06.787 I/InputResult: INPUT_RESULT_HANDLED
9:05:06.803 I/apz.inputqueue: Line 185: Will consume touch event
9:05:06.803 I/InputResult: INPUT_RESULT_HANDLED_CONTENT
9:05:06.819 I/apz.inputqueue: Line 185: Will consume touch event
9:05:06.820 I/InputResult: INPUT_RESULT_HANDLED_CONTENT
9:05:06.836 I/apz.inputqueue: Line 185: Will consume touch event
9:05:06.836 I/InputResult: INPUT_RESULT_HANDLED_CONTENT
...

I tried to follow with logs what happens in nsWindow.cpp based on nsEventStatus_eConsumeDoDefault but it seems like this doesn't reach nsWindow.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE

Sorry, I copy-pasted the title but forgot to modify it to express the issue.
This problem is similar to the one in bug 1631754.
.. Just that we get the reverse. INPUT_RESULT_HANDLED_CONTENT instead of the expected INPUT_RESULT_HANDLED on some specific sites.
Not sure if this will also be resolved by bug 1631754.
If you think this is still a dupe please close it.

Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: PanZoomController erroneously returns INPUT_RESULT_HANDLED → PanZoomController erroneously returns INPUT_RESULT_HANDLED_CONTENT

(In reply to Petru-Mugurel Lingurar[:petru] from comment #0)

Seen while browsing github.com

This is the root page on github.com, or some specific subpage? And are you logged in or out? (you get different content if you are logged in)

Flags: needinfo?(petru.lingurar)

Just tried it on geckoview-example. Based on reports from here https://github.com/mozilla-mobile/fenix/issues/9720 I see the issue reproducing on:

On all of these sites the page scroll works fine and scrolls are expected (because of the visuals) to be handled by Gecko and such PZC to return INPUT_RESULT_HANDLED.
But in the majority of cases the return is INPUT_RESULT_HANDLED_CONTENT.
Strangely, scrolling from the same positions (in page) sometimes may return INPUT_RESULT_HANDLED for the entire scroll gesture, but most of the times (even from the same starting position if tried again) the return is INPUT_RESULT_HANDLED_CONTENT.

Flags: needinfo?(petru.lingurar)
Priority: -- → P1
Whiteboard: [
Whiteboard: [ → [geckoview:m78]

The reason for this is that the pages have apz-aware listeners on the root element. For github.com, there is a touchmove listener registered on the <html> element which is the root scroller.

Thank you for looking into this.
So as I understand PZC works as intended here even though we actually scrolled the page.

We needed this in Fenix to properly differentiate if the browser / the website consumed the scroll to correctly animate pull down to refresh / the urlBar.
What intrigues me is that I see both this and bug 1631754 not being issues on Fennec which correctly animates the AwesomeBar and shows the overflow effect so that makes me wonder if we should better use those apis..

There's two different things you're trying to do here: animate the dynamic toolbar, and implement pull to refresh.

For the dynamic toolbar, I don't think you need to rely much on whether or not content used the event. If you rely on that you open yourself up to an attack vector where the page can consume the events, prevent the Fenix dynamic toolbar from becoming visible, and then spoof its own image of the dynamic toolbar. That's a security bug we worked hard to avoid in previous implementations. So I think you should be able to have a decent dynamic toolbar implementation if you pretty much ignore what APZ is doing and just drive the show/hide behaviour based on user input.

For the pull-to-refresh, the situation is quite different. In that case you only want the behaviour to occur if you know for sure the event wasn't consumed by APZ or content. That information can't be obtained just by the return value from ReceiveInputEvent, because at that point APZ itself doesn't necessarily know what the input event triggered. The input event may be handled asynchronously after waiting for content to run JS and so on. So IMO to handle pull-to-refresh properly you need an API similar to overscroll - after APZ/content is done with its stuff you would get a callback saying "this is the unused amount of scroll that occurred" and then you can decide if you want to use it for pull-to-refresh or not.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7)

There's two different things you're trying to do here: animate the dynamic toolbar, and implement pull to refresh.

For the dynamic toolbar, I don't think you need to rely much on whether or not content used the event. If you rely on that you open yourself up to an attack vector where the page can consume the events, prevent the Fenix dynamic toolbar from becoming visible, and then spoof its own image of the dynamic toolbar. That's a security bug we worked hard to avoid in previous implementations. So I think you should be able to have a decent dynamic toolbar implementation if you pretty much ignore what APZ is doing and just drive the show/hide behaviour based on user input.

I don't agree with this. You have to care what APZ/Gecko is doing because for things like Google Maps you want to suspend any toolbar changes while the user is panning the map around. The Fennec toolbar animator was intimately tied to what APZ/Gecko were doing.

The spoofing problem is easily fixed by showing the toolbar when an ACTION_DOWN event was not used for a toplevel pan/zoom. Fenix is doing this today.

The toolbar is a probably the only place where having a separation of app vs gecko creates a problem. The dynamic toolbar animator from Fennec wasn't chosen for this because I felt that the screenshot hack was too gross to be the right API for GV. In Fennec it was more justifiable.

For the pull-to-refresh, the situation is quite different. In that case you only want the behaviour to occur if you know for sure the event wasn't consumed by APZ or content. That information can't be obtained just by the return value from ReceiveInputEvent, because at that point APZ itself doesn't necessarily know what the input event triggered. The input event may be handled asynchronously after waiting for content to run JS and so on. So IMO to handle pull-to-refresh properly you need an API similar to overscroll - after APZ/content is done with its stuff you would get a callback saying "this is the unused amount of scroll that occurred" and then you can decide if you want to use it for pull-to-refresh or not.

Yeah, overscroll might be a better fit for pull-to-refresh. I have a patch that hooks the glow effect back up, we could also then expose that stuff to apps for their own usage.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #8)

I don't agree with this. You have to care what APZ/Gecko is doing because for things like Google Maps you want to suspend any toolbar changes while the user is panning the map around. The Fennec toolbar animator was intimately tied to what APZ/Gecko were doing.

I might have oversimplified a bit. It's certainly true that the Fennec toolbar was more tied to APZ/Gecko. But I'm still confused how allowing Google Maps to suspend toolbar changes doesn't open you up to the spoofing problem. Imagine a malicious map website where if you pan the map upwards, it displays a fake URL bar at the bottom of the screen. A user goes the page, pans down to the map (which hides the real URL bar) and then pans around on the map. User sees what appears to be the URL bar reappearing, and uses it. But it's actually the spoofed bar and they get hijacked.

The spoofing problem is easily fixed by showing the toolbar when an ACTION_DOWN event was not used for a toplevel pan/zoom. Fenix is doing this today.

In the scenario I described above does panning the map count as "an ACTION_DOWN not used for a toplevel pan/zoom"? If so isn't this heuristic ("show the toolbar when an ACTION_DOWN is not used for a toplevel pan/zoom") inherently conflicting with "suspend toolbar changes during map panning"?

The toolbar is a probably the only place where having a separation of app vs gecko creates a problem. The dynamic toolbar animator from Fennec wasn't chosen for this because I felt that the screenshot hack was too gross to be the right API for GV. In Fennec it was more justifiable.

I agree with you there, I'm glad to see the screenshot hack go.

Yeah, overscroll might be a better fit for pull-to-refresh. I have a patch that hooks the glow effect back up, we could also then expose that stuff to apps for their own usage.

Sounds good.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #8)

I don't agree with this. You have to care what APZ/Gecko is doing because for things like Google Maps you want to suspend any toolbar changes while the user is panning the map around. The Fennec toolbar animator was intimately tied to what APZ/Gecko were doing.

I might have oversimplified a bit. It's certainly true that the Fennec toolbar was more tied to APZ/Gecko. But I'm still confused how allowing Google Maps to suspend toolbar changes doesn't open you up to the spoofing problem. Imagine a malicious map website where if you pan the map upwards, it displays a fake URL bar at the bottom of the screen. A user goes the page, pans down to the map (which hides the real URL bar) and then pans around on the map. User sees what appears to be the URL bar reappearing, and uses it. But it's actually the spoofed bar and they get hijacked.

The spoofing problem is easily fixed by showing the toolbar when an ACTION_DOWN event was not used for a toplevel pan/zoom. Fenix is doing this today.

In the scenario I described above does panning the map count as "an ACTION_DOWN not used for a toplevel pan/zoom"?

Yes, as we'd return HANDLED_CONTENT in that case.

If so isn't this heuristic ("show the toolbar when an ACTION_DOWN is not used for a toplevel pan/zoom") inherently conflicting with "suspend toolbar changes during map panning"?

Well by "suspend" I meant "don't show/hide the toolbar based on touch movement". Given your example above, we would immediately show the urlbar with the first touch event that went to the map. While the user pans the map around, the toolbar would stay "pinned". Any fake urlbar would appear above the real one.

Ok, I see. So then either we need to integrate more deeply with APZ or we need to be ok with getting some false positives. In the latter case we should also ensure we only return HANDLED when we're absolutely sure content isn't controlling outcomes, which means doing what Botond suggested here.

Assignee: nobody → kats
Attachment #9151241 - Attachment is obsolete: true
Assignee: kats → nobody
Status: REOPENED → RESOLVED
Closed: 5 years ago → 4 years ago
Resolution: --- → DUPLICATE

This is not a duplicate of bug 1640387. It's the opposite problem.

Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

We are seeing a similar behaviour on the setting page of UBlock origin. As Petru-Mugurel mentioned in comment #0. GV returns INPUT_RESULT_HANDLED_CONTENT signalling the web page handled the scroll and not the browser, causing that we do not animate(hide/show the toolbar) and the bottom part of the page is not getting shown, As you can see in this video. More information can be found in the Fenix related bug

Hey Emily, what does the timeline look like for this? I believe this is the last outstanding issue before we can turn on dynamic toolbar, which vesta wants for release. Ideally, if we could get this into GV78 by the end of next week (6/18), that should make our sprint, but I guess that All Hands is also that week...

https://github.com/mozilla-mobile/fenix/issues/8775 is our meta

Flags: needinfo?(etoop)

:kats, is this something that you are looking at? Fenix are hoping to have this fix uplifted to 78.

Flags: needinfo?(etoop) → needinfo?(kats)
Whiteboard: [geckoview:m78] → [geckoview:m78][geckoview:m79]

No, I'm not actively looking at this. But reading through the comments again I'm not sure exactly what the problem is anymore. If we are returning HANDLED_CONTENT instead of HANDLED in some cases (because of event listeners or whatever), then presumably it should be triggering the behaviour that :snorp described here:

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #10)

we would immediately show the urlbar with the first touch event that went to the map. While the user pans the map around, the toolbar would stay "pinned".

So from a user point of view what I would expect (e.g. in this video) is that the URL bar appears and remains pinned visible, rather than distractingly scrolling on or off. :snorp, can you confirm my interpretation is correct? If so maybe this is a simple GV-side fix.

If we want that return value to be more precise and return HANDLED in cases where the event listeners didn't do a preventDefault then we will need tighter integration between APZ and GV and in particular some sort of callback mechanism instead of a synchronous return value, so that will be trickier to implement.

Flags: needinfo?(kats) → needinfo?(snorp)

That's correct, if we return HANDLED_CONTENT for the ACTION_DOWN event then the toolbar should be pinned visible for the duration of the touch. I can't reproduce the behavior shown in the video from comment #18 anymore. The toolbar stays visible the entire time.

Flags: needinfo?(snorp)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #19)

That's correct, if we return HANDLED_CONTENT for the ACTION_DOWN event then the toolbar should be pinned visible for the duration of the touch. I can't reproduce the behavior shown in the video from comment #18 anymore. The toolbar stays visible the entire time.

Whoops, I didn't see that the video there was using a top toolbar. The issue there is that Fenix is not correctly pinning the toolbar in the HANDLED_CONTENT case.

OK, I think we want to try to pursue the path where we wait to see what Gecko did with the event. Kats / Botond can you guess at when you could get to this? Is it possible for a mortal to write the patch?

Flags: needinfo?(kats)
Flags: needinfo?(botond)

Would an API along the following lines work for GV?

  • The APZEventResult contains a Maybe<mTargetIsRoot> which is populated if we know the answer for sure, and empty if we don't.
  • If it's empty, GV takes note of the mInputBlockId in the APZEventResult as identifying an input block for which it needs to receive a delayed answer.
  • When the delayed answer is ready, APZ calls a function with a (uint64_t inputBlockId, bool targetIsRoot) pair that tells GV the answer for that input block.

It should be fairly straightforward to arrange for APZ to call such a function when a delayed answer is ready.

Flags: needinfo?(botond)
Flags: needinfo?(kats)

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

Would an API along the following lines work for GV?

  • The APZEventResult contains a Maybe<mTargetIsRoot> which is populated if we know the answer for sure, and empty if we don't.
  • If it's empty, GV takes note of the mInputBlockId in the APZEventResult as identifying an input block for which it needs to receive a delayed answer.
  • When the delayed answer is ready, APZ calls a function with a (uint64_t inputBlockId, bool targetIsRoot) pair that tells GV the answer for that input block.

It should be fairly straightforward to arrange for APZ to call such a function when a delayed answer is ready.

That sounds fine. I think we'd only want to do it in certain cases where we actually care about the result, though (which for Fenix would only be ACTION_DOWN events), otherwise I feel like it could potentially generate too much traffic.

I'll give implementing the APZ side of this a try.

Assignee: nobody → botond

GeckoView uses these flags for the same purpose: to determine
whether or not to allow certain effects, such as pull to refresh
or dynamic toolbar movement, that should be limited to cases
where an event manipulates the root scrollable.

In a subsequent patch, we will enhance the API between APZ and
GeckoView such that in cases where we don't know for sure whether
an event will manipulate the root scrollable until it is
dispatched to content, we tell GV the answer asynchronously.
This is easier if there is only one flag to report.

This facility is only implemented for in-process senders of
input events (i.e. not for GPU process setups).

Depends on D79930

The posted patches implement the APZ side of the proposed API. The last patch is a starting point for where to implement the GV side. Let me know if this is suitable!

Flags: needinfo?(snorp)
Whiteboard: [geckoview:m78][geckoview:m79] → [geckoview:m78][geckoview:m79][geckoview:m80]

Checked the dependencies for pull to refresh on Fenix.
I still see this being a problem for the websites from this list (other than Github) that also affects the animation for the bottom toolbar.

Though testing on w3schools I see different behaviors (based on different pzc results) - video so maybe all of this can also be seen as webcompat issues?

Also seeing the Fennec overscroll effect working as expected even on these pages.

Flags: needinfo?(snorp)
Whiteboard: [geckoview:m78][geckoview:m79][geckoview:m80] → [geckoview:m78][geckoview:m79][geckoview:m80][geckoview:m81]
Blocks: 1649865

Botond, I have a patch here[1] that uses the new input block callback. I seem to be getting handled = true every time we go down this path, though. Have I done something wrong? For instance, on maps.google.com the touch events should be handled in content so I would think it would return false....

[1] https://phabricator.services.mozilla.com/D86384

Flags: needinfo?(botond)
Attachment #9157167 - Attachment is obsolete: true

The early-return path in the AddInputBlockCallback() case still needs to perform the window->ProcessUntransformedAPZEvent() stuff, as that's what actually dispatches the event to content. With that change to your patch, Google Maps seems to be working fine (and producing aHandledByRootApzc = false).

Flags: needinfo?(botond)
Assignee: botond → snorp
Attachment #9168832 - Attachment description: Bug 1633322 - Handle asynchronous responses for motion events → Bug 1633322 - Consider content handling for `onTouchEventForResult`
Pushed by jwillcox@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/193fa2a0b926 Combine APZEventResult::mTargetIsRoot and mHitRegionWithApzAwareListeners into a single flag. r=kats,geckoview-reviewers,snorp https://hg.mozilla.org/integration/autoland/rev/21c0baa65724 Turn APZEventResult::mHandledByRootApzc into a Maybe. r=kats,geckoview-reviewers,snorp https://hg.mozilla.org/integration/autoland/rev/8271d4e51f25 Add an input block callback facility to allow senders of input events to receive a delayed notification of whether the event was processed by the root APZC. r=kats https://hg.mozilla.org/integration/autoland/rev/ffa257a29649 Consider content handling for `onTouchEventForResult` r=geckoview-reviewers,botond,agi,esawin

Backed out for android failures e.g. test_group_checkerboarding.html

backout: https://hg.mozilla.org/integration/autoland/rev/3117c5a77009b0a5ba9647b0c55b98cd17d23f4f

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=ffa257a296496a5640d227fabd52315ef83fbc55&searchStr=Android&selectedTaskRun=PAQQyWZHQmeALUel2i-KWg.0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=313353389&repo=autoland&lineNumber=6251

[task 2020-08-18T20:17:32.093Z] 20:17:32 INFO - 3633 INFO TEST-START | gfx/layers/apz/test/mochitest/test_group_checkerboarding.html
[task 2020-08-18T20:22:58.948Z] 20:22:58 INFO - Buffered messages logged at 20:17:26
[task 2020-08-18T20:22:58.948Z] 20:22:58 INFO - 3634 INFO TEST-PASS | gfx/layers/apz/test/mochitest/test_group_checkerboarding.html | Starting subtest helper_checkerboard_apzforcedisabled.html
[task 2020-08-18T20:22:58.948Z] 20:22:58 INFO - Buffered messages finished
[task 2020-08-18T20:22:58.949Z] 20:22:58 WARNING - 3635 INFO TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_group_checkerboarding.html | Test timed out.
[task 2020-08-18T20:22:58.950Z] 20:22:58 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:417:16
[task 2020-08-18T20:22:58.950Z] 20:22:58 INFO - reportError@SimpleTest/TestRunner.js:143:22
[task 2020-08-18T20:22:58.950Z] 20:22:58 INFO - TestRunner._checkForHangs@SimpleTest/TestRunner.js:165:18
[task 2020-08-18T20:22:58.950Z] 20:22:58 INFO - 3636 INFO TEST-OK | gfx/layers/apz/test/mochitest/test_group_checkerboarding.html | took 324285ms

also org.mozilla.geckoview.test.PanZoomControllerTest.touchEventForResult | java.lang.AssertionError: Value should match https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=313354872&repo=autoland&lineNumber=8174

Flags: needinfo?(snorp)

Botond, I have a fix for the mochitest failure, but the junit failure seems to be intermittent and I was hoping you could help me figure that out. The part that's failing is that sometimes ACTION_DOWN on an element with a touchstart listener returns result.mHandledByRootApzc == Some(true) and result.mStatus == nsEventStatus_eIgnore. Is APZ not aware of the touch region in this case? Where should I go poking around?

Flags: needinfo?(snorp) → needinfo?(botond)
Pushed by jwillcox@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9be548b05e86 Combine APZEventResult::mTargetIsRoot and mHitRegionWithApzAwareListeners into a single flag. r=kats,geckoview-reviewers,snorp https://hg.mozilla.org/integration/autoland/rev/1aa6be497177 Turn APZEventResult::mHandledByRootApzc into a Maybe. r=kats,geckoview-reviewers,snorp https://hg.mozilla.org/integration/autoland/rev/84454fa520be Add an input block callback facility to allow senders of input events to receive a delayed notification of whether the event was processed by the root APZC. r=kats https://hg.mozilla.org/integration/autoland/rev/953d128f4c51 Consider content handling for `onTouchEventForResult` r=geckoview-reviewers,botond,agi,esawin

Backed out 5 changesets (bug 1633322, bug 1634504) for touchEventForResult gv-junit failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=EcSDJ46BRV2-_lITvl2zkA.0&fromchange=472c546742470cbf78cc794fddd7e5a65a609ace&searchStr=gv-junit&tochange=184dbd9b95ebc4185c683693ace4f46e443e69a8

Backout link: https://hg.mozilla.org/integration/autoland/rev/184dbd9b95ebc4185c683693ace4f46e443e69a8

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

[task 2020-08-19T23:26:18.330Z] 23:26:18     INFO -  TEST-START | org.mozilla.geckoview.test.PanZoomControllerTest.touchEventForResult
[task 2020-08-19T23:26:18.530Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=756
[task 2020-08-19T23:26:18.530Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2020-08-19T23:26:18.530Z] 23:26:18     INFO -  org.mozilla.geckoview.test | Error in touchEventForResult(org.mozilla.geckoview.test.PanZoomControllerTest):
[task 2020-08-19T23:26:18.530Z] 23:26:18     INFO -  org.mozilla.geckoview.test | java.lang.AssertionError: Value should match
[task 2020-08-19T23:26:18.530Z] 23:26:18     INFO -  org.mozilla.geckoview.test | Expected: <1>
[task 2020-08-19T23:26:18.530Z] 23:26:18     INFO -  org.mozilla.geckoview.test |      but: was <0>
[task 2020-08-19T23:26:18.531Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
[task 2020-08-19T23:26:18.531Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.Assert.assertThat(Assert.java:956)
[task 2020-08-19T23:26:18.531Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.rules.ErrorCollector$1.call(ErrorCollector.java:65)
[task 2020-08-19T23:26:18.531Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.rules.ErrorCollector.checkSucceeds(ErrorCollector.java:78)
[task 2020-08-19T23:26:18.531Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.rules.ErrorCollector.checkThat(ErrorCollector.java:63)
[task 2020-08-19T23:26:18.531Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.checkThat(GeckoSessionTestRule.java:802)
[task 2020-08-19T23:26:18.532Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.BaseSessionTest.assertThat(BaseSessionTest.kt:88)
[task 2020-08-19T23:26:18.532Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PanZoomControllerTest.touchEventForResult(PanZoomControllerTest.kt:280)
[task 2020-08-19T23:26:18.532Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Native Method)
[task 2020-08-19T23:26:18.532Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
[task 2020-08-19T23:26:18.532Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
[task 2020-08-19T23:26:18.533Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
[task 2020-08-19T23:26:18.533Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
[task 2020-08-19T23:26:18.533Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2.lambda$evaluate$0$GeckoSessionTestRule$2(GeckoSessionTestRule.java:1302)
[task 2020-08-19T23:26:18.533Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.-$$Lambda$GeckoSessionTestRule$2$sIbRNaZJgAu-QrUVWSGD8JbPSWM.run(lambda)
[task 2020-08-19T23:26:18.533Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1950)
[task 2020-08-19T23:26:18.533Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.handleCallback(Handler.java:751)
[task 2020-08-19T23:26:18.534Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2020-08-19T23:26:18.534Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.os.Looper.loop(Looper.java:154)
[task 2020-08-19T23:26:18.534Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.app.ActivityThread.main(ActivityThread.java:6077)
[task 2020-08-19T23:26:18.534Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Native Method)
[task 2020-08-19T23:26:18.534Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
[task 2020-08-19T23:26:18.534Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
[task 2020-08-19T23:26:18.534Z] 23:26:18     INFO -  org.mozilla.geckoview.test |
[task 2020-08-19T23:26:18.535Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2020-08-19T23:26:18.535Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=touchEventForResult
[task 2020-08-19T23:26:18.535Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.PanZoomControllerTest
[task 2020-08-19T23:26:18.535Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stack=java.lang.AssertionError: Value should match
[task 2020-08-19T23:26:18.535Z] 23:26:18     INFO -  org.mozilla.geckoview.test | Expected: <1>
[task 2020-08-19T23:26:18.535Z] 23:26:18     INFO -  org.mozilla.geckoview.test |      but: was <0>
[task 2020-08-19T23:26:18.535Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
[task 2020-08-19T23:26:18.536Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.Assert.assertThat(Assert.java:956)
[task 2020-08-19T23:26:18.536Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.rules.ErrorCollector$1.call(ErrorCollector.java:65)
[task 2020-08-19T23:26:18.536Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.rules.ErrorCollector.checkSucceeds(ErrorCollector.java:78)
[task 2020-08-19T23:26:18.536Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.rules.ErrorCollector.checkThat(ErrorCollector.java:63)
[task 2020-08-19T23:26:18.536Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.checkThat(GeckoSessionTestRule.java:802)
[task 2020-08-19T23:26:18.536Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.BaseSessionTest.assertThat(BaseSessionTest.kt:88)
[task 2020-08-19T23:26:18.537Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PanZoomControllerTest.touchEventForResult(PanZoomControllerTest.kt:280)
[task 2020-08-19T23:26:18.537Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Native Method)
[task 2020-08-19T23:26:18.537Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
[task 2020-08-19T23:26:18.537Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
[task 2020-08-19T23:26:18.537Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
[task 2020-08-19T23:26:18.537Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
[task 2020-08-19T23:26:18.538Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2.lambda$evaluate$0$GeckoSessionTestRule$2(GeckoSessionTestRule.java:1302)
[task 2020-08-19T23:26:18.538Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.-$$Lambda$GeckoSessionTestRule$2$sIbRNaZJgAu-QrUVWSGD8JbPSWM.run(lambda)
[task 2020-08-19T23:26:18.538Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1950)
[task 2020-08-19T23:26:18.538Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.handleCallback(Handler.java:751)
[task 2020-08-19T23:26:18.538Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2020-08-19T23:26:18.538Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.os.Looper.loop(Looper.java:154)
[task 2020-08-19T23:26:18.539Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at android.app.ActivityThread.main(ActivityThread.java:6077)
[task 2020-08-19T23:26:18.539Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Native Method)
[task 2020-08-19T23:26:18.539Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
[task 2020-08-19T23:26:18.539Z] 23:26:18     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
[task 2020-08-19T23:26:18.539Z] 23:26:18     INFO -  org.mozilla.geckoview.test |
[task 2020-08-19T23:26:18.539Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=389
[task 2020-08-19T23:26:18.539Z] 23:26:18     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
[task 2020-08-19T23:26:18.540Z] 23:26:18  WARNING -  TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.PanZoomControllerTest.touchEventForResult | java.lang.AssertionError: Value should match
[task 2020-08-19T23:26:18.540Z] 23:26:18     INFO -  TEST-INFO took 208ms
Flags: needinfo?(snorp)

Ugh. Looks like I need to wait for onFirstContentfulPaint() in setupScroll() as well.

Flags: needinfo?(snorp)
Pushed by jwillcox@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9025714892ac Combine APZEventResult::mTargetIsRoot and mHitRegionWithApzAwareListeners into a single flag. r=kats,geckoview-reviewers,snorp https://hg.mozilla.org/integration/autoland/rev/a0291bc39a66 Turn APZEventResult::mHandledByRootApzc into a Maybe. r=kats,geckoview-reviewers,snorp https://hg.mozilla.org/integration/autoland/rev/3953149e9b4a Add an input block callback facility to allow senders of input events to receive a delayed notification of whether the event was processed by the root APZC. r=kats https://hg.mozilla.org/integration/autoland/rev/29e16ac5d3de Consider content handling for `onTouchEventForResult` r=geckoview-reviewers,botond,agi,esawin
Regressions: 1660357
Flags: needinfo?(botond)

Backed out from Beta 81 at Snorp's request to allow more time for downstream consumers to adapt. It remains landed for 82+.
https://hg.mozilla.org/releases/mozilla-beta/rev/c3e9617c37d69d2064e4a83015a304048f5cc7ed

status-firefox81: fixed → wontfix
status-firefox82: --- → fixed

Just wanted to say that I tested
https://www.phonearena.com
https://www.w3schools.com
on Fenix and the issue seem fixed - the toolbar is now correctly animated.

Thank you!

Depends on: 1662823
See Also: → 1740739

Moving some input bugs to the new GeckoView::IME component.

Component: General → IME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: