Closed Bug 1116586 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Layout, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: kats, Assigned: kats)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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
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.
Assignee: nobody → bugmail.mozilla
Attachment #8542688 - Attachment is obsolete: true
Attachment #8551865 - Flags: review?(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.
Attachment #8551865 - Flags: review?(tnikkel) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/c248261b202e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
No longer blocks: parent-process-apz
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
Attachment #8551865 - Flags: approval-mozilla-b2g37?
Attachment #8551865 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
You need to log in before you can comment on or make changes to this bug.