Closed Bug 1495470 Opened 2 years ago Closed 2 years ago

Google Photos' left sidebar menu items (e.g. "Albums") are unclickable, if CSS Containment is enabled


(Core :: Layout, defect, P1)




Tracking Status
firefox64 --- fixed


(Reporter: dholbert, Assigned: dholbert)




(3 files, 1 obsolete file)

 1. Set the pref layout.css.contain.enabled to true
 2. Visit (and log in if you're not logged in)
 3. Try to hover and/or click the icons on the left sidebar (Photos, Albums, Assistant, Sharing, Photo books)

 - The cursor doesn't change when I hover these icons.
 - Clicks on the icons have no effect.

 - The cursor should change to reflect the fact that they're clickable.

If I right-click the icons and do "inspect" to see what's intercepting the mouse events, I can see that the clicks are being blocked by a large element <c-wiz ...>, which:
 - ...has padding-left:96px, which is the area that these icons are "shining through"
 - ...has "contain:layout style"

If I remove "layout" from its contain property, then I get "expected results". So something about contain:layout is making this element intercept clicks in its padding area.
Actually there are several <c-wiz> elements involved here, and I think its a more ancesterly one whose 'contain' styles are breaking things here.

Coming up with a reduced testcase...
Attached file testcase 1
Attachment #9013366 - Attachment is obsolete: true
Attachment #9013366 - Attachment description: testcase 1 → testcase 1 (obsolete/replaced due to typo in close tag)
testcase 2 is pretty reduced. It shows a red box in Firefox, vs. a green box in Chrome.
So right now, containment is having an effect in the testcase. And the fact that it's having an effect is a bug.

* "containment is having an effect": testcase 2 renders red in Nightly with layout.css.contain.enabled = true, but it renders as false if you turn off the pref or remove the 'contain:layout' styling.

* "that's a bug": The span here is a non-atomic inline-level box, and that means (per spec) the "contain:layout" styling should have no effect:
> ...if the element’s principal box is a non-atomic inline-level box, layout containment has no effect.

The testcase involves an IB split... I suspect our "should we honor layout containment" code (for determining stacking contexts perhaps) is wrong, for IB splits.
Yeah, so with content like...
   <span style="contain:layout">   <!--- (or contain:paint) -->

...we end up generating a frame tree that looks like:
>  line <
>    Inline(span)
>  >
>  line <
>    Block(span) <
>      line <
>        Block(div)
>      >
>    >
>  >
>  line <
>    Inline(span)
>  >

...and notably, the IB-split-generated block ("Block(span)") has the span's styles applied to it. So it has contain:layout, and  so since it's block-level, it thinks it should generate a stacking context.

But really, it shouldn't generating a stacking context, because the span's "principal box is a non-atomic inline-level box" (which means its 'contain' styling shouldn't matter).
It looks like the problem is just that nsIFrame::IsStackingContext() calls IsContainLayout() and IsContainPaint() without testing whether the frame supports them.

I think we need to add this check there, as a precondition for caring about IsContainLayout/IsContainPaint:

If I add that, it fixes this.
Assignee: nobody → dholbert
(Looks like the Try run shows that this makes us pass two annotated-as-failing WPT tests, too -- contain-layout-018.html and contain-paint-025.html. I'll remove the .ini expected-fail notations for those.)
Pushed by
Only let 'contain:layout/paint' create stacking contexts on frames that support it. r=dbaron
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.