Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Description • 10 years ago

STR:
1. Turn on layout.event-regions.enabled
2. Go to people.mozilla.com/~kgupta/griddiv.html and scroll the div

Expected:
div is scrollable

Actual:
div is not scrollable

Regression from bug 1082594. What's happening is that the scrollframe for the div is active, but isn't getting event regions populated correctly. Prior to bug 1082594 we would use the scroll info layer's composition bounds to make event regions and so it worked, but bug 1082594 removed that code and so now the underlying bug is exposed.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 2 • 10 years ago

Half of this change is what tn suggested at https://bugzilla.mozilla.org/show_bug.cgi?id=1104099#c5, so that the event regions are associated with the correct layer. The other half is so that the event regions aren't just thrown away - I'm not sure that I'm adding it to the right list but it seems close enough to what the other users of nsDisplayLayerEventRegions do. Without this it looks like once the AutoBuildingDisplayList [1] gets popped off the stack at [2], the event regions are just thrown away because they never get added to an actual display list.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp?rev=e497957b79b2#2374
[2] http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp?rev=e497957b79b2#2446

	Timothy Nikkel (:tnikkel)
	Comment 3 • 10 years ago

Comment on attachment 8528558 [details] [diff] [review]
Part 2 - Ensure the event regions for non-stacking children are kept

Oops!

	Timothy Nikkel (:tnikkel)
	Comment 4 • 10 years ago

Comment on attachment 8528555 [details] [diff] [review]
Part 1 - Log which event region objects get accumulated into which layers

>+  void AccumulateEventRegions(nsDisplayLayerEventRegions* aEventRegions)
>   {
>-    mHitRegion.Or(mHitRegion, aHitRegion);
>-    mMaybeHitRegion.Or(mMaybeHitRegion, aMaybeHitRegion);
>-    mDispatchToContentHitRegion.Or(mDispatchToContentHitRegion, aDispatchToContentHitRegion);
>+    FLB_LOG_PAINTED_LAYER_DECISION(this, "Accumulating regions %p against tld=%p\n", aEventRegions, this);

Call them event regions in the log for clarity maybe?

I guess we should change tld to pld at some point in this debug logging too. (But not here)

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 5 • 10 years ago

(In reply to Timothy Nikkel (:tn) from comment #4)
> Call them event regions in the log for clarity maybe?

Done

> I guess we should change tld to pld at some point in this debug logging too.
> (But not here)

Yeah I'll file a bug for that.

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/d0c4d779391b
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/5a77aacfd82e

	Carsten Book [:Tomcat]
	Comment 6 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/d0c4d779391b
https://hg.mozilla.org/mozilla-central/rev/5a77aacfd82e