Closed
Bug 1207157
Opened 9 years ago
Closed 9 years ago
block formatting context pushed down below float when only its right margin doesn't fit
Categories
(Core :: Layout: Floats, defect)
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox41 | --- | unaffected |
firefox42 | --- | affected |
firefox43 | --- | fixed |
firefox44 | --- | fixed |
People
(Reporter: pimschreurs, Assigned: dbaron)
References
Details
Attachments
(4 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jfkthame
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jfkthame
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150913004016
Steps to reproduce:
See the test case attached to this bug report.
Actual results:
Block B is now below block A
Expected results:
Block B should be next to block A. This is what happens in Firefox 40.0.3 (and Chrome 45 and IE11). I'll admit that I'm not sure this is what actually should have happened, but it happens in the other browsers, so I figured it was a bug.
Comment 1•9 years ago
|
||
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ed8935b903ed&tochange=c63a6810b2bb
Triggered by: Bug 478834
Blocks: 478834
status-firefox41:
--- → unaffected
status-firefox42:
--- → affected
status-firefox43:
--- → affected
status-firefox44:
--- → affected
Component: Untriaged → Layout: Floats
Flags: needinfo?(dbaron)
Product: Firefox → Core
Comment 2•9 years ago
|
||
So in that testcase, the container has a width of 442px. Block A and B both have width 49%, so 216.58px. That adds up to 433.16px. They also have right margins of 5px each, which puts you at 443.16px, which is more than 442px.
Now I guess the question is whether the 5px margin of B sticking out on the right should cause B to get line-wrapped...
Note that before bug 478834 was fixed we simply didn't move B down in this case even if it totally didn't fit (e.g. if the width was set to 51% instead of 49% in this testcase).
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #2)
> Now I guess the question is whether the 5px margin of B sticking out on the
> right should cause B to get line-wrapped...
Chrome appears to be ignoring the right margin.
I think there are parts of this code in which we ignore the margins as well -- but apparently not this part.
All http://www.w3.org/TR/CSS21/visuren.html#bfc-next-to-float says is:
The border box of a table, a block-level replaced element, or an element in the normal flow that establishes a new block formatting context (such as an element with 'overflow' other than 'visible') must not overlap the margin box of any floats in the same block formatting context as the element itself. If necessary, implementations should clear the said element by placing it below any preceding floats, but may place it adjacent to such floats if there is sufficient space. They may even make the border box of said element narrower than defined by section 10.3.3. CSS2 does not define when a UA may put said element next to the float or by how much said element may become narrower.
which isn't actually helpful.
Assignee | ||
Updated•9 years ago
|
Summary: overflow:hidden no longer prevents an element from wrapping → block formatting context pushed down below float when only its right margin doesn't fit
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to David Baron [:dbaron] ⌚UTC-7 from comment #3)
> Chrome appears to be ignoring the right margin.
IE11 does the same. So I guess we should match.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → dbaron
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 5•9 years ago
|
||
Flags: needinfo?(dbaron)
Assignee | ||
Comment 6•9 years ago
|
||
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8666296 -
Flags: review?(jfkthame)
Assignee | ||
Comment 8•9 years ago
|
||
The change to the 427129-table-caption test was just to make the
reference match the test.
The change to the other 427129 tests was duplicating the test that
failed, adjusting the reference for one version of it (no longer pushed
down), and testing an alternative pushed-down case for the other
version.
It's not clear to me how to test that that
1062963-floatmanager-reflow.html still tests what it originally tested,
given the code change that's happened since.
Attachment #8666297 -
Flags: review?(jfkthame)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8666298 -
Flags: review?(jfkthame)
Updated•9 years ago
|
Attachment #8666296 -
Flags: review?(jfkthame) → review+
Updated•9 years ago
|
Attachment #8666297 -
Flags: review?(jfkthame) → review+
Updated•9 years ago
|
Attachment #8666298 -
Flags: review?(jfkthame) → review+
Comment 10•9 years ago
|
||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d589a5de39ad
https://hg.mozilla.org/mozilla-central/rev/2c3577c59382
https://hg.mozilla.org/mozilla-central/rev/b002f72aa051
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8666297 [details] [diff] [review]
patch 2 - Stop caring about a replaced element's margin-inline-end when determining whether it fits next to floats
Approval Request Comment
[Feature/regressing bug #]: longstanding bug that was made more observable by bug 478834 (Firefox 42)
[User impact if declined]: incorrect layout of some Web pages
[Describe test coverage new/current, TreeHerder]: has reftests
[Risks and why]: reasonably low risk since it's improving compatibility with other browsers
[String/UUID change made/needed]: no
Attachment #8666297 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•9 years ago
|
Attachment #8666298 -
Flags: approval-mozilla-aurora?
Comment on attachment 8666297 [details] [diff] [review]
patch 2 - Stop caring about a replaced element's margin-inline-end when determining whether it fits next to floats
Approved for uplift, has reftests, fixes an issue that became apparent in 42.
Attachment #8666297 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8666298 [details] [diff] [review]
patch 3 - Don't bother passing around the inline-end margin of replaced elements we consider clearing past floats, since we don't need it any more
Approved for uplift to aurora.
Attachment #8666298 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Patch 1 with the reftests should also uplift to aurora, please.
Comment 16•9 years ago
|
||
Assignee | ||
Comment 17•9 years ago
|
||
(In reply to David Baron [:dbaron] ⌚️UTC-7 from comment #3)
> (In reply to Boris Zbarsky [:bz] from comment #2)
> > Now I guess the question is whether the 5px margin of B sticking out on the
> > right should cause B to get line-wrapped...
>
> Chrome appears to be ignoring the right margin.
Actually, I think this analysis was wrong; I'm not sure what the condition that causes Chrome to push blocks down or not is.
You need to log in
before you can comment on or make changes to this bug.
Description
•