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

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect)

Description

[sirxemic]

Attachment: 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

[Alice0775 White]

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

Triggered by: Bug 478834

Comment 2

[Boris Zbarsky [:bzbarsky]]

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).

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d36a1de9d654

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3efb26a207c

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 1 - Add reftest

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2 - Stop caring about a replaced element's margin-inline-end when determining whether it fits next to floats

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.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: 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

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d589a5de39ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c3577c59382
https://hg.mozilla.org/integration/mozilla-inbound/rev/b002f72aa051

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d589a5de39ad
https://hg.mozilla.org/mozilla-central/rev/2c3577c59382
https://hg.mozilla.org/mozilla-central/rev/b002f72aa051

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Patch 1 with the reftests should also uplift to aurora, please.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/743dd7362602
https://hg.mozilla.org/releases/mozilla-aurora/rev/92261c345518
https://hg.mozilla.org/releases/mozilla-aurora/rev/c72a8bcab579

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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.