Closed
Bug 531371
Opened 14 years ago
Closed 14 years ago
background image disappears when body{display:table} is used (common CSS hack for correct centering+scrolling in Firefox)
Categories
(Core :: Layout: Tables, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta5-fixed |
People
(Reporter: jc.bugzilla.20.jc, Assigned: roc)
References
()
Details
(Keywords: regression)
Attachments
(2 files)
819 bytes,
text/html
|
Details | |
10.14 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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 1•14 years ago
|
||
Gecko/20090721 Minefield/3.6a1pre http://hg.mozilla.org/mozilla-central/rev/f2a58ffcd00c fails Gecko/20090722 Minefield/3.6a1pre http://hg.mozilla.org/mozilla-central/rev/02f8bf10f441 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f2a58ffcd00c&tochange=02f8bf10f441 I suspect bug 505184.
![]() |
||
Comment 2•14 years ago
|
||
There should be a thin lime strip at the top of the page.
![]() |
||
Comment 3•14 years ago
|
||
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
![]() |
||
Comment 4•14 years ago
|
||
And in fact bug 505184's use of DisplayBackgroundUnconditional would cause just such behavior.
Component: Layout: Images → Layout: Tables
QA Contact: layout.images → layout.tables
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → roc
Assignee | ||
Comment 5•14 years ago
|
||
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.
Attachment #415055 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•14 years ago
|
||
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.
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs review]
![]() |
||
Comment 7•14 years ago
|
||
> 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•14 years ago
|
||
Comment on attachment 415055 [details] [diff] [review] fix Looks good.
Attachment #415055 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs review] → [needs landing]
Updated•14 years ago
|
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Flags: blocking1.9.2+
![]() |
||
Comment 9•14 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/4f6f09ec4447
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
![]() |
||
Comment 10•14 years ago
|
||
Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/556b68fc2b33
status1.9.2:
--- → final-fixed
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs landing]
You need to log in
before you can comment on or make changes to this bug.
Description
•