550882 - Setting overflow=hidden and then overflow=visible using script we get no scrollbars back

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Alice0775 White]

Attachment: testcase

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.2pre) Gecko/20100307 Namoroka/3.6.2pre ID:20100307043156
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.2pre) Gecko/20100307 Namoroka/3.6.2pre ID:20100307043156

Setting overflow=hidden and then overflow=visible using script we get no scrollbars back.
Original report is http://forums.mozillazine.org/viewtopic.php?f=9&t=1781275 

Firefox3.0.x and 3.5.x work well.
But Firefox 3.6.0 and Minefield 3.7a3pre fails.

Is the change of this behavior the intention of the developer?
or something regression?

Steps to Reproduce:
1. Open testcase
2. Click "overflow:visible" button
3. 

Actual Results:
 no scrollbars appear.

Expected Results:
 scrollbars should appear.

regression window:

works:
http://hg.mozilla.org/mozilla-central/rev/02f8bf10f441
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090722 Namoroka/3.6a1pre ID:20090722042136

fails:
http://hg.mozilla.org/mozilla-central/rev/d783d91beec5
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090723 Namoroka/3.6a1pre ID:20090723043404

pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=02f8bf10f441&tochange=d783d91beec5

There is same issue on latest trunk nightly.
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100307 Minefield/3.7a3pre ID:20100307041909

oter browsers:
Firefox 3.0.x and 3.5.x:  works as expected.
Opera 10.50: works as expected.
Google chrome 4.0.249.89: after step 2, resoze window then  scrollbars appear.
Safari 4.0.4:  after step 2, resoze window then  scrollbars appear.
IE8: as same as Firefox3.6.x

Comment 1

[Alice0775 White]

Sorry, I revised spelling.  
s/oter/other/ s/resoze/resize/g

other browsers:
Firefox 3.0.x and 3.5.x:  works as expected.
Opera 10.50: works as expected.
Google chrome 4.0.249.89: after step 2, resize window then  scrollbars appear.
Safari 4.0.4:  after step 2, resize window then  scrollbars appear.
IE8: as same as Firefox3.6.x

Comment 2

[Boris Zbarsky [:bzbarsky]]

Looks like a regression from bug 502447, which is pretty odd.  Looking into this now.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So what used to happen in this testcase before the fix for bug 502447 landed is that when we constructed the root scrollframe we didn't see the overflow:hidden and hence gave it scrollbars.  Then at layout time we saw they should be hidden and didn't show them.

With the patch for bug 502447 we now see the body styling during the root frame construction and therefore don't create scrollbars at all.  Then at layout time we might want to show them, but they're just not there.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: This fixes it.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/c7c0db9074c7

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 431182 [details] [diff] [review]
This fixes it.

Should probably get this in on branch too.  Not sure we need this for 1.9.2.2, though.

Comment 7

[Boris Zbarsky [:bzbarsky]]

This was backed out on suspicion of causing a Tsunspider regression.  Backing out this patch and bug 503832 did fix that regression, an I just relanded this patch as http://hg.mozilla.org/mozilla-central/rev/f0239b3789d9 to try to narrow down which of the two is responsible for the slowdown.

Comment 8

[Boris Zbarsky [:bzbarsky]]

And backed out again; it definitely causes the Tsspider regression.

Comment 10

[Boris Zbarsky [:bzbarsky]]

I relanded as http://hg.mozilla.org/mozilla-central/rev/b3367021d661 a few days ago with an attempt to make the code flow identical with and without the patch ... and it _still_ regressed Tsspider.  Since then Tsspider has been updated to not measure pageload anymore, so I just pushed http://hg.mozilla.org/mozilla-central/rev/23e09c2ded59 to remove the hackery.  Calling this fixed.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Christian, this is a somewhat serious web compat regresion that's been waiting on approval for over a month now...  I understand not wanting to block on it, but can we just get it checked in on the branch?

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think we should block on it; it's a regression of pretty basic functionality.

Comment 14

[LegNeato]

This is too late for 1.9.2.4. We'll take this in 1.9.2.5

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 431182 [details] [diff] [review]
This fixes it.

Approved for 1.9.2.5, a=dveditz for release-drivers

Comment 16

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cc47b92e73d8

I hope setting this as fixed1.9.2.6 is the right thing to do.