1275411 - [e10s] css with nested hidden and visible styling has regressed in firefox beta v48.0a2 since version 46.0.1

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Rodney Draaisma]

Attachment: testbug.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160521140538

Steps to reproduce:

I created a popout panel that consists of alternating hidden and visible items see the attached file.

If you load the attached file in version 48 beta, no popout panel is showing.
While loading it in an older versions of firefox or in any other browser it will show the popout panel.
If you want to see what it should look like, open the attached html file in Firefox 46.0.1, chrome/chromium, ie 11 or edge


Actual results:

Popout field is not showing on firefox 48 beta, but does show on all other browsers including firefox 46.0.1


Expected results:

Show the popout field.

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]: layout is broken due to regression

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c525c5164cdde373a636d525e53c77f52d8fd693&tochange=c99e8abb5e00eaa439a3be7864df0e4135326349

Suspect : Bug 1231538

Comment 3

[Timothy Nikkel (:tnikkel)]

I can reproduce in old nightly builds, but this is fixed for me in current nightly. Presumably because of bug 1265237.

Comment 4

[Alice0775 White]

(In reply to Timothy Nikkel (:tnikkel) from comment #3)
> I can reproduce in old nightly builds, but this is fixed for me in current
> nightly. Presumably because of bug 1265237.

NO.
I can still reproduce on Nightly49.0a1 [1] with e10s.

[1]
https://hg.mozilla.org/mozilla-central/rev/d6d4e8417d2fd71fdf47c319b7a217f6ace9d5a5
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0 ID:20160525063710

Comment 5

[Timothy Nikkel (:tnikkel)]

Oops, my mistake, I didn't test current nightly with e10s.

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: reduced testcase

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: patch + test

When we are saving oof data for the outer fixed frame we don't save any oof data because the fixed frame has zero size since it doesn't contain anything. So when we descend into the outer fixed frame via it's placeholder later we reset the clip state, but leave the scroll clip state alone. The scrollclip from the empty overflow: hidden div is providing the scroll clip. That scroll clip is the one that is current when we later create the nsDisplayFixedPos item.

Only shows up on e10s cause we only create nsDisplayFixedPos items when there is a display port. Creating nsDisplayFixedPos items when there is fixed pos without a displayport lets you reproduce on non-e10s.

I was _just_ thinking that we should clear the scroll clip here, but I managed to convince myself that we didn't need to. Oops.

Comment 8

[Timothy Nikkel (:tnikkel)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d39ffc0e743

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29637a385bc2

Comment 10

[Marcia Knous [:marcia]]

Let's track this layout regression for 48/49.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/29637a385bc2

Comment 12

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8756652 [details] [diff] [review]
patch + test

Approval Request Comment
[Feature/regressing bug #]: bug 1231538
[User impact if declined]: some popup menu designs using fixed pos won't render
[Describe test coverage new/current, TreeHerder]: added a test
[Risks and why]: low risk
[String/UUID change made/needed]: none

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8756652 [details] [diff] [review]
patch + test

Regression in e10s scenario, Aurora48+

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Rodney, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 15

[Rodney Draaisma]

I can confirm this is fixed on version 49.0a1 (2016-05-27)

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/65badb6bff1e

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fc30c000238
Actually enable reftest.

Comment 18

[Timothy Nikkel (:tnikkel)]

(In reply to Pulsebot from comment #17)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/0fc30c000238
> Actually enable reftest.

The reftest.list entry addition must have gotten lost when I failed to do a rebase correctly.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0fc30c000238