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)

42 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox41 --- unaffected
firefox42 --- affected
firefox43 --- fixed
firefox44 --- fixed

People

(Reporter: pimschreurs, Assigned: dbaron)

References

Details

Attachments

(4 files)

Attached file Test case (deleted) —
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.
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
Attached patch patch 1 - Add reftest (deleted) — Splinter Review
Attachment #8666296 - Flags: review?(jfkthame)
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?
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.
Depends on: 1236745
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.

Attachment

General

Creator:
Created:
Updated:
Size: