Closed Bug 1059167 Opened 6 years ago Closed 6 years ago

Right floated element with defined width doesn't respect right margin


(Core :: Layout: Floats, defect, P2, major)




Tracking Status
firefox31 --- unaffected
firefox32 --- unaffected
firefox33 --- unaffected
firefox34 + fixed


(Reporter: azasypkin, Assigned: jfkthame)



(Keywords: regression, testcase)


(3 files, 1 obsolete file)

Attached file test-case.html
While working on bug 1041765, we've spotted blocking Gecko regression - right floated elements can't have margin-right (see attached test case).

Everything is fine for left floated elements. For right floated elements margin has effect only if element has "width: auto;".

The issue DOES occur on the latest Nightly and B2G, and DOES NOT occur on the week-old builds.
[Blocking Requested - why for this release]:
Blocks v2.1 blocker
blocking-b2g: --- → 2.1?
First changeset that fails the test:

changeset:   201250:54ada5ad66bb
user:        Jonathan Kew <>
date:        Sun Aug 24 15:34:44 2014 +0100
summary:     bug 1046950 pt 2 - convert ComputeSize to use logical-coordinate parameters. r=smontagu
Blocks: 1046950
Keywords: testcase
OS: Linux → All
Priority: -- → P2
Hardware: x86_64 → All
This is a regression from bug 1046950 as far as I can tell.

Specifically, in the code in FloatMarginWidth() was changed from passing the margin, border, and padding to ComputeSize() to passing margin, borderpadding, and padding.  So the padding gets double-counted as far as I can tell.  Removing the padding style in the testcase makes the margin work correctly.
Flags: needinfo?(smontagu)
Flags: needinfo?(jfkthame)
Keywords: testcase
OS: All → Linux
Priority: P2 → --
Hardware: All → x86_64
Keywords: testcase
OS: Linux → All
Priority: -- → P2
Hardware: x86_64 → All
Yes, it's a regression; oops. Will fix and test.

(FWIW, removing "box-sizing: border-box;" from the testcase style also avoids the issue.)
Flags: needinfo?(smontagu)
Flags: needinfo?(jfkthame)
This was an unintended change during logical-conversion; we weren't supposed to drop the subtraction of the padding here. I'll see about writing a reftest that would've caught this...
Attachment #8479906 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Attached patch reftest. (obsolete) — Splinter Review
Simple reftest based on the testcase here.
Attachment #8479920 - Flags: review?(smontagu)
Attached patch reftest.Splinter Review
It works better to actually include the test files in the patch.
Attachment #8480048 - Flags: review?(smontagu)
Attachment #8479920 - Attachment is obsolete: true
Attachment #8479920 - Flags: review?(smontagu)
Tryserver job with the reftest:
Duplicate of this bug: 1059633
Attachment #8479906 - Flags: review?(smontagu) → review+
Attachment #8480048 - Flags: review?(smontagu) → review+
Duplicate of this bug: 1059845
Duplicate of this bug: 1059809
FWIW: The newrelic blog ( , ) is broken in current Nightly, I think by this bug.  (main text content overflowing & getting clipped on the right side)

Glad that this is fixed on inbound; hopefully that means the newrelic blog will be working again tomorrow or the next day. :)
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
blocking-b2g: 2.1? → ---
You need to log in before you can comment on or make changes to this bug.