block formatting context pushed down below float when only its right margin doesn't fit

RESOLVED FIXED in Firefox 43

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: pimschreurs, Assigned: dbaron)

Tracking

42 Branch
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 unaffected, firefox42 affected, firefox43 fixed, firefox44 fixed)

Details

Attachments

(4 attachments)

Reporter

Description

4 years ago
Posted file Test case —
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

4 years ago
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ed8935b903ed&tochange=c63a6810b2bb

Triggered by: Bug 478834
Blocks: 478834
Component: Untriaged → Layout: Floats
Flags: needinfo?(dbaron)
Product: Firefox → Core
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).
(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.
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
(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: nobody → dbaron
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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)
Attachment #8666296 - Flags: review?(jfkthame) → review+
Attachment #8666297 - Flags: review?(jfkthame) → review+
Attachment #8666298 - Flags: review?(jfkthame) → review+
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?
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.

Updated

3 years ago
Depends on: 1236745

Updated

3 years ago
No longer depends on: 1236745
(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.