Last Comment Bug 531371 - background image disappears when body{display:table} is used (common CSS hack for correct centering+scrolling in Firefox)
: background image disappears when body{display:table} is used (common CSS hack...
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: unspecified
: x86 All
: -- major (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
http://www.webdesignerwall.com/tutori...
Depends on:
Blocks: 505184
  Show dependency treegraph
 
Reported: 2009-11-27 03:14 PST by jc.bugzilla.20.jc
Modified: 2010-01-06 17:00 PST (History)
5 users (show)
dbaron: blocking1.9.2+
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta5-fixed


Attachments
test case (819 bytes, text/html)
2009-11-27 16:39 PST, philippe (part-time)
no flags Details
fix (10.14 KB, patch)
2009-11-29 17:54 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
bzbarsky: review+
Details | Diff | Review

Description jc.bugzilla.20.jc 2009-11-27 03:14:35 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9.2b4) Gecko/20091124 Firefox/3.6b4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9.2b4) Gecko/20091124 Firefox/3.6b4

Firefox 3.6 Beta doesnt show background images on pages which use
body {
  background: url(image.jpg) no-repeat scroll center top;
  display: table;
}
to get a centered background image which is centered on the webpage.
(omiting display:table makes the background image centered as if there would be no scrolling)

Reproducible: Always

Steps to Reproduce:
1. open http://www.webdesignerwall.com/tutorials/how-to-css-large-background/ and compare it with FF3.5
2. open Firebug on this page (with FF3.6) and disable body {display:table}

Actual Results:  
1. FF3.6 doesnt show the background image, FF3.5 does
2. FF3.6 also shows the background image, but with the scrolling problem

Expected Results:  
display the backround image as in versions before

( OR: at least show the background image as there would be no display:table )

http://www.webdesignerwall.com/tutorials/how-to-css-large-background/ also explains why this CSS Hack is needed in Firefox.

--

I classified this bug as a major bug because it breaks existing webpages and requires the webmasters to use a CSS switch if this bug will be contained in FF3.6 final version.
Comment 2 philippe (part-time) 2009-11-27 16:39:25 PST
Created attachment 414885 [details]
test case

There should be a thin lime strip at the top of the page.
Comment 3 Boris Zbarsky [:bz] 2009-11-27 21:51:54 PST
It looks like we're not longer propagating body backgrounds to the canvas at all in this case, no?  I mean... the background color isn't painting either.
Comment 4 Boris Zbarsky [:bz] 2009-11-27 21:58:39 PST
And in fact bug 505184's use of DisplayBackgroundUnconditional would cause just such behavior.
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-11-29 17:54:18 PST
Created attachment 415055 [details] [diff] [review]
fix

I'm not sure what you meant about DisplayBackgroundUnconditional being the problem. It seems to me that the bug is that FindRootFrame (which needs a better name) doesn't look for the inner table frame when the body element's primary frame is a tableOuter frame. So this patch creates nsLayoutUtils::GetStyleFrame and uses it in FindRootFrame and in a couple of other places where we're explicitly checking for tableOuter right now.

Then again, I must be missing something since I don't see how this could have worked before.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-11-29 18:16:38 PST
I just realized that we don't properly handle the case where <body> is display:none. I guess we should still propagate the background in that case. But that requires more significant changes, because we can't just return a frame. Right now I have difficulty caring.
Comment 7 Boris Zbarsky [:bz] 2009-11-30 11:53:49 PST
> I'm not sure what you meant about DisplayBackgroundUnconditional being the
> problem.

I think I was just wrong about that.

> I don't see how this could have worked before.

It "worked" by not propagating the background to the viewport (e.g. the color doesn't cover the whole viewport if set) but just painting it on the table.  The new code skips painting via the table painter and puts in an nsDisplayBackground.  That calls nsCSSRendering::PaintBackground, which calls FindBackground, which calls FindElementBackground, which returns false, since the root element's background is transparent and this is the body.  Then aForFrame has no -moz-appearance, and in any case it's not for the root element, so we bail out of PaintBackground.
Comment 8 Boris Zbarsky [:bz] 2009-11-30 11:54:17 PST
Comment on attachment 415055 [details] [diff] [review]
fix

Looks good.
Comment 9 Boris Zbarsky [:bz] 2009-12-01 09:22:48 PST
Pushed http://hg.mozilla.org/mozilla-central/rev/4f6f09ec4447
Comment 10 Boris Zbarsky [:bz] 2009-12-01 09:56:05 PST
Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/556b68fc2b33

Note You need to log in before you can comment on or make changes to this bug.