Closed
Bug 398144
Opened 17 years ago
Closed 17 years ago
inline-block should always grow to contain floats
Categories
(Core :: Layout, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta2
People
(Reporter: nikolakocicbz, Assigned: roc)
References
()
Details
(Keywords: regression, testcase, Whiteboard: [dbaron-1.9:RwCo])
Attachments
(5 files, 2 obsolete files)
1.17 KB,
text/html
|
Details | |
758 bytes,
text/html
|
Details | |
248 bytes,
text/html
|
Details | |
1.35 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
2.48 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007092705 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007092705 Minefield/3.0a9pre ID:2007092705
Layout on http://www.anime-planet.com/ is messed up but it's displayed normally in IE, Opera and FF2.
Reproducible: Always
Reporter | ||
Comment 2•17 years ago
|
||
Tried with
Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1)
and
Opera/9.20 (Windows NT 6.0; U; en)
no changes
Yeah but if you change the user agent from within Minefield to say Firefox does it work then? That is what I meant.
I tested and it does not look different if I change what I was talking about above.
Can you create a reduced testcase?
Reporter | ||
Comment 4•17 years ago
|
||
Sorry, I meant I was using User Agent Switcher (from Minefield) with those strings and there was no change.
About testcase, sorry but I can't do anything about that, I just noticed an error and after consulting with site admin (he told me that it's almost certainly Minefield bug) I decided to fill bug here and to hope it gets fixed... I hope someone can help.
Can you find the regression range on which was the last build it looked correctly and the first one it didn't look correctly?
Reporter | ||
Comment 6•17 years ago
|
||
Found it:
2007-01-27-04-works
2007-01-28-04-broken
yeah, it was broken back in 3.0a2 times...
Comment 7•17 years ago
|
||
Heh. I've just found the same range.. mostly because it looks like the same thing I filed bug 392182 about. If the problem is due to inline-block, then this should probably be a dupe of 392182.
Reporter | ||
Comment 8•17 years ago
|
||
Hmm... Bug 392182's testcase is same in current Minefield and Opera (but in Opera even when you zoom-in there's no difference) but it's correctly displayed in IE.
www.anime-planet.com is same in Opera and IE.
Not sure if it makes a difference though...
Comment 9•17 years ago
|
||
Just to be clear with my range, works:
- Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a2pre) Gecko/2007012602 Minefield/3.0a2pre
Broken:
- Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a2pre) Gecko/2007012715 Minefield/3.0a2pre
Which is a day's difference to the range found in comment 6, but the same range as bug 392182
Reporter | ||
Comment 10•17 years ago
|
||
May I ask where did you got that hourly builds from? I can only find nightlies (xxxx-xx-xx-04) on mozilla ftp server. And it's possible that regression was between 2007012704 and 2007012715, that way both your and mine ranges are correct...
Comment 11•17 years ago
|
||
Yes, our regression ranges do actually overlap, which is good news! But in future, it would help if you could paste the whole user agent string ( from about: ) rather than just a date, because different OS builds will be at different hours.
Yes, the mozilla archive only keeps the official nightlies. The builds I used were from my own archive of hourly and nightly builds I've downloaded and saved.
![]() |
||
Comment 12•17 years ago
|
||
WEbKit and Safari 3 display that page correctly (not overlap of content).
In Minefield, the offending element (div id=homeintro) has indeed display:inline-block, and that is the cause of the problem. But I don't think Minefield is behaving correctly. Not sure what the problem is though.
![]() |
||
Comment 13•17 years ago
|
||
The top block and bottom block are identical but for one thing: the inline-block has a border at the bottom: it displays correctly. In the top block, the inline-block collapses: it does not contain the floats correctly (it should).
![]() |
||
Updated•17 years ago
|
Blocks: inline-block
![]() |
||
Comment 14•17 years ago
|
||
Additionally, the site in the URL field has this (a common technique to clear floats), but it has no effect on inline-block.
div#homeintro:after {
display:block;
clear:both;
content:"";
height:0;
}
----------
This is a pretty annoying problem, as 'display:inline-block' is regularly used as bug fixer (it triggers 'hasLayout') in IE 6 and 7 Windows.
Severity: normal → major
You mean 'clear' has no effect *after* an inline-block? It's not supposed to have an effect on an inline-block [1], nor should it have any effect at all in this testcase. However, the first half of the testcase is showing a bug where the inline-block ends up with the incorrect size. It establishes a new block formatting context and therefore should grow to contain floats [2]. I'm not sure why it's doing so only some of the time here. (I'm assuming the first and second halves of the simplified testcase differ only by a border, though I might have missed something.)
[1] http://www.w3.org/TR/CSS21/visuren.html#propdef-clear and
http://www.w3.org/TR/CSS21/visuren.html#block-boxes
[2] http://www.w3.org/TR/CSS21/visuren.html#block-formatting and
http://www.w3.org/TR/CSS21/visudet.html#root-height
Flags: blocking1.9?
Summary: Layout messed up at http://www.anime-planet.com/ → inline-block should always grow to contain floats
Or did you intend comment 14 to be about a separate bug?
![]() |
||
Comment 17•17 years ago
|
||
(In reply to comment #15)
In the minimised test case, the top and bottom only differ by the border. Add a border and the inline-block contains the floats (as it should do anyway, as it establishes a block formatting context, per note [2] in your comment). And note, only a bottom-border is needed.
And no, I did not mean to imply that the clear property applied to an element that follows the inline-block should have an effect on floats inside an inline-block.
(in the test case, the clear:both applied to .u-block, I left it there as it was part of the original)
(In reply to comment #16)
> Or did you intend comment 14 to be about a separate bug?
>
I noted the ::after 'hack' in the original stylesheet as it explains why the site displays correctly with Gecko 1.8.
But the ::after generated content applies _inside_ the inline-block, and should clear the floats _inside_ the inline-block. That might actually work correctly I think (but is only visible if there is real content: ::after {content:"xxxxxxxxxx"}).
I haven't played much with the combination of inline-block and generated content to know it there are any problems with it. I'll investigate, and file bugs if needed.
![]() |
||
Comment 18•17 years ago
|
||
some clean up, some more colour to visualise what is going on.
Note that the inline-block actually grows to contain the float(s) (I added some padding to the left in the inline-block, in lime-green). The clearing doesn't work properly: the yellow block must be below the line-green block.
Attachment #283121 -
Attachment is obsolete: true
Comment 19•17 years ago
|
||
Simpler testcase
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → roc
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Whiteboard: [dbaron-1.9:RwCo]
Assignee | ||
Comment 20•17 years ago
|
||
What's happening here is that given
<div style="display:inline-block;"><div style="float:left;">left float</div></div>
The inline-block is deciding that it's "empty" --- because all its in-flow contents are empty --- and therefore its block container gives its line zero advance height.
I think inline-blocks should be treated as non-empty, always, like replaced elements.
Assignee | ||
Comment 21•17 years ago
|
||
Simple fix
Attachment #285840 -
Flags: superreview?(dbaron)
Attachment #285840 -
Flags: review?(dbaron)
It seems like anything that has a space manager (as inline blocks do) should also check its float list for emptyness before declaring that it's empty. Though this fix may also be good (are there good ways to test this in other browsers? and does the spec define emptiness for any of the things we use IsEmpty for?), and may make that more general fix not matter.
How about some reftests?
Assignee | ||
Comment 23•17 years ago
|
||
(In reply to comment #22)
> It seems like anything that has a space manager (as inline blocks do) should
> also check its float list for emptyness before declaring that it's empty.
That's an option, I guess.
> Though this fix may also be good (are there good ways to test this in other
> browsers?
I'll have a go.
> and does the spec define emptiness for any of the things we use
> IsEmpty for?),
http://www.w3.org/TR/CSS21/box.html
Our IsEmpty controls whether to collapse top and bottom margins for an element, and the spec says to collapse top and bottom margins for an element when they're "adjoining", where:
An element's own margins are adjoining if the 'min-height' property is zero, and it has neither top or bottom borders nor top or bottom padding, and it has a 'height' of either 0 or 'auto', and it does not contain a line box, and all of its in-flow children's margins (if any) are adjoining.
It also says:
Margins of inline-block elements do not collapse (not even with their in-flow children).
*If* completely trimmed-away text does not cause creation of a line box, according to the spec, then this corresponds quite closely to IsEmpty(). In that case I think I can interpret the spec as implying inline-block elements (and any other margin-root!) are not empty... Maybe I should test for margin-root-ness instead of inline-block, making anything that's a margin-root non-empty?
> How about some reftests?
Easy, I'll do some when I check the fix in.
Assignee | ||
Comment 24•17 years ago
|
||
This test tests emptiness of inline-block. When an inline-block is treated as empty, the margins of its parent DIV collapse together and the gap between the blue DIVs is 100px; this is what trunk does. When it's treated like a replaced element, the margins of the parent DIV do not collapse together and the gap is 200px. This is what we do with my patch, and it's what Safari 3 does.
Assignee | ||
Comment 25•17 years ago
|
||
Apparently IE7 also gives 200px.
I'll revise my patch to check for margin-root-ness. That seems like a very logical way to solve this.
Assignee | ||
Comment 26•17 years ago
|
||
Attachment #285840 -
Attachment is obsolete: true
Attachment #285855 -
Flags: superreview?(dbaron)
Attachment #285855 -
Flags: review?(dbaron)
Attachment #285840 -
Flags: superreview?(dbaron)
Attachment #285840 -
Flags: review?(dbaron)
Comment on attachment 285855 [details] [diff] [review]
updated fix
r+sr=dbaron
Attachment #285855 -
Flags: superreview?(dbaron)
Attachment #285855 -
Flags: superreview+
Attachment #285855 -
Flags: review?(dbaron)
Attachment #285855 -
Flags: review+
Assignee | ||
Updated•17 years ago
|
Whiteboard: [dbaron-1.9:RwCo] → [dbaron-1.9:RwCo][needs landing]
Assignee | ||
Updated•17 years ago
|
Priority: -- → P2
Comment 28•17 years ago
|
||
Checking in layout/generic/nsBlockFrame.cpp;
/cvsroot/mozilla/layout/generic/nsBlockFrame.cpp,v <-- nsBlockFrame.cpp
new revision: 3.883; previous revision: 3.882
done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [dbaron-1.9:RwCo][needs landing] → [dbaron-1.9:RwCo]
Target Milestone: --- → mozilla1.9 M10
Assignee | ||
Comment 29•17 years ago
|
||
I'll check this in when I get a chance
Comment 31•17 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050810 Minefield/3.0pre
Verified.
You need to log in
before you can comment on or make changes to this bug.
Description
•