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

Status: RESOLVED DUPLICATE of bug 1224015

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: 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

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Option (c) - Use the event target rather than redoing the hit-test

Comment 3

[Botond Ballo [: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?

Comment 4

[Kartikaya Gupta (email:[e-mail])]

We ended up doing option (a) and putting it in bug 1224015 (attachment 8689841 [details] [diff] [review], specifically). Duping this over.