1116586 - Event regions do not get properly generated for items with rounded corners

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

For items with rounded corners, the area of the item is added to the "maybe-hit" region at [1]. This later gets added to the dispatch-to-content region on the layer, at [2] (but not to the hit region). However, the interface contract of the event regions [3] implies that the dispatch-to-content region must be a subregion of the hit region. This is not the case for items with rounded corners.

This is one of the contributing factors that makes the edge-gesture-swipe touch areas in the b2g chrome process [4] have improper hit regions.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp?rev=3f980229dfc1#3062
[2] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=190df6248c74#2299
[3] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/Layers.h?rev=0743f344f853#914
[4] https://github.com/mozilla-b2g/gaia/blob/26d479f0fccb7174e06255121e4e938c1b280d67/apps/system/index.html#L1213-L1214

Comment 1

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

Attachment: WIP

Comment 2

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

We will need this (and bug 1116588) before we can land bug 950934, otherwise it becomes possible to scroll content while in the middle of an edge swipe gesture which (while kinda neat) is a regression.

Comment 3

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

Attachment: Expand the hit region to completely cover the maybe-hit region

Comment 4

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

Try is green https://treeherder.mozilla.org/#/jobs?repo=try&revision=a864c2e776f5

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8551865 [details] [diff] [review]
Expand the hit region to completely cover the maybe-hit region

It's very confusing to have two things called mHitRegion, and we pass regions from one to the other, but they mean very different things. In fact it's the reason we have this bug.

The layer hit region means the union of the definitely hit region and the dispatch to content (to decide what to do) region.

The layout hit region means "definitely hit" region.

I wish I had a better idea off the top of my head for how to avoid this.

Comment 6

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

(In reply to Timothy Nikkel (:tn) from comment #5)
> I wish I had a better idea off the top of my head for how to avoid this.

If you think of anything let me know.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c248261b202e

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c248261b202e

Comment 8

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

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> We will need this (and bug 1116588) before we can land bug 950934, otherwise
> it becomes possible to scroll content while in the middle of an edge swipe
> gesture which (while kinda neat) is a regression.

Actually bug 1123599 indicates that it's possible to scroll content while in the middle of an edge swipe gesture since bug 920036, so this dependency moves up.

Comment 9

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

Comment on attachment 8551865 [details] [diff] [review]
Expand the hit region to completely cover the maybe-hit region

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): this is needed for bug 930939 which is 2.2+
User impact if declined: touch listeners registered on elements with rounded corners (border-radius is set) cannot properly prevent scrolling. this happens for instance with the edge swipe gesture (see bug 1123599).
Testing completed: locally
Risk to taking this patch (and alternatives if risky): low risk, patch is small and well understood
String or UUID changes made by this patch: none

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4047ab9ee909