Closed Bug 1104099 Opened 5 years ago Closed 5 years ago

Non-visible PaintedLayer ends up with a big hit region, consumes all events

Categories

(Core :: Layout, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(3 files)

STR:
1. Run the latest B2G code with the layout.event-regions.enabled pref set to true.
2. Go into the settings app and try to scroll

Expected:
Settings app is scrollable

Actual:
Settings app is not scrollable

Looking at the layer dump (attached) I see a layer (0xb1767400) sitting on top of the scrollable layer for the settings app (0xb0e5b400) and having a hit region that consumes all of the touch events. This results in none of the touch events getting to the scrollable layer and so the user can't scroll it.
With the patches on bug 1104266 applied, I see that of the 5 LayerEventRegions display items at the end of the dump, the first four have a hit region of (x=0, y=0, w=19200, h=32360) and the fifth one is empty.

tn, do you have any suggestions on what I can try here?
Flags: needinfo?(tnikkel)
The display list dump doesn't have the empty painted layers in it?

Anyways, my guess for what is happening is that all the display items get assigned to painted layers based on their animated geometry roots, and the layer event regions items get assigned to a painted layer that doesn't have any actual display items that paint things assigned to it.
Flags: needinfo?(tnikkel)
The display list dump does have the empty painted layer, 0xb3db4090 (it was a different run from the layer dump).
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> The display list dump does have the empty painted layer, 0xb3db4090 (it was
> a different run from the layer dump).

Thanks, I must have read it too quickly.

I still think comment 3 is what's happening. I think this code in nsIFrame::BuildDisplayListForChild http://dxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#2423 is what makes this happen. Specficially we create a nsDisplayLayerEventRegions with this as the underlying frame, and then we add the child's area to the hit region with AddFrame. The child can (and will sometimes) have a different animated geometry root. The nsDisplayLayerEventRegions should probably be using the child as the underlying frame there.
Unfortunately that didn't seem to fix it.

I dug into this further and it looks like in the settings app there are a few <section> elements with class="dialog", e.g. [1], [2]. As a result, these section elements have a different computed style. In particular, the transform:translateX(100%) that the other section elements have is replaced with transform:unset, and there is an additional z-index:3 property. I expect that one of these two changes is responsible for the layout code picking up a hit region from these elements. I think this is a layout bug because the elements do also have visibility:hidden and so should not be receiving input events.

[1] https://github.com/mozilla-b2g/gaia/blob/eb54eb137e84d7d40618dd3757bdfb2342d91b1c/apps/settings/index.html#L333
[2] https://github.com/mozilla-b2g/gaia/blob/eb54eb137e84d7d40618dd3757bdfb2342d91b1c/apps/settings/index.html#L400
Oh, if they have visibility hidden we should probably just not AddFrame them to the layer event regions.
Attached patch PatchSplinter Review
Assignee: nobody → bugmail.mozilla
Attachment #8528456 - Flags: review?(tnikkel)
Attachment #8528456 - Flags: review?(mstange)
Comment on attachment 8528456 [details] [diff] [review]
Patch

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

::: layout/base/nsDisplayList.cpp
@@ +2960,5 @@
>    }
> +  if (!aFrame->StyleVisibility()->IsVisible()) {
> +    return;
> +  }
> +  printf_stderr("staktrace: adding frame %p to regions %p\n", aFrame, this);

Whoops, I'll remove the printf.
Attachment #8528456 - Flags: review?(mstange) → review+
Attachment #8528456 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/75152d829ec8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.