Hit test used for layerization doesn't work properly in Fennec with C++ APZ

RESOLVED DUPLICATE of bug 1224015

Status

()

Core
Panning and Zooming
RESOLVED DUPLICATE of bug 1224015
2 years ago
2 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

With the various apz-fennec patches applied (in particular attachment 8689551 [details] [diff] [review]), if you load http://bluemarvin.github.io/html/divscroll.html and try to scroll the div, putting your finger down near the top of the div, it won't scroll. What's happening is that the div doesn't get layerized because the code in PrepareForSetTargetAPZCNotification doesn't find the inner scrollable frame; it just ends up hitting the RCDRSF. This in turn is because the resolution isn't getting unapplied correctly.

One possible fix (and perhaps the "correct" fix) is to resurrect something like attachment 8649403 [details] [diff] [review] so that the hit testing unapplies the resolution during the hit test. Because without that, calling GetFrameForPoint on the root document will never work perfectly in the case where the RCD has a resolution - either it will work for things in the root document, or it will work for things in the root content document, but not both at the same time.

Another option is to just start from the root content document rather than the root document in that hunk of code, but we'd probably need to ifdef that for fennec so as to not break other platforms. I implemented this and it seems to work but it's not a great solution.

A third option (which I implemented, and seems to work) is to not do that hit-test at all, and instead use the event target from the Widget*Event which has already been dispatched through gecko and has done a hit test. This has the advantage of also avoiding an extra display list build.
Created attachment 8689749 [details] [diff] [review]
Option (b) - use the RCD instead of the RD

This is the middle option in comment 0, using the RCD instead of the RD for the layerization hit-test so that it works. Stashing it here because I don't want to lose the code
Created attachment 8689752 [details] [diff] [review]
Option (c) - Use the event target rather than redoing the hit-test
Assignee: nobody → bugmail.mozilla
Attachment #8689752 - Flags: review?(botond)
Comment on attachment 8689752 [details] [diff] [review]
Option (c) - Use the event target rather than redoing the hit-test

Review of attachment 8689752 [details] [diff] [review]:
-----------------------------------------------------------------

First, this doesn't work on TabChild platforms, because TabChild::RecvRealTouchEvent() calls SendSetTargetAPZCNotification() _before_ dispatching the event to content, so the targets will not have been set yet.

Second, doing a hit-test instead of using the event targets was a conscious choice made for reasons described in bug 918288 comment 61. Are those reasons no longer valid?
Attachment #8689752 - Flags: review?(botond) → review-
We ended up doing option (a) and putting it in bug 1224015 (attachment 8689841 [details] [diff] [review], specifically). Duping this over.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1224015
You need to log in before you can comment on or make changes to this bug.