Top margin on table doesn't allow it to clear a previous float

RESOLVED FIXED in mozilla10

Status

()

Core
Layout: Tables
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: ataylor32, Assigned: Ehsan)

Tracking

({dev-doc-complete, testcase})

Trunk
mozilla10
x86
Windows 7
dev-doc-complete, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 ( .NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 ( .NET CLR 3.5.30729)

The tables are all way off to the right. Firefox is the one and only browser I've seen with this problem, so I assume the problem is with Firefox and not with the CSS code. I confirmed that this problem still exists in version 3.7a5pre.

Reproducible: Always

Steps to Reproduce:
Simply visit the URL.

Comment 1

7 years ago
Created attachment 443753 [details]
reduced testcase

Behaves different in any other browser

Updated

7 years ago
Component: General → Layout: Floats
Product: Firefox → Core
QA Contact: general → layout.floats
If I give the float 'line-height: 1' it behaves as you expect (though even then I don't think it's a safe assumption that it should).  With line-height: normal (which is the font's normal line height, generally greater than 1) this is the behavior I'd expect.
Or perhaps the difference is that other browsers are checking the space below the margin-top of the table?
Hmm.  The spec requires the border box of the table to not overlap the float, but doesn't say anything about the margin box not being allowed to overlap.

I think the issue is that even after fixing bug 478614 the table's margin is on the _inner_ table, not the outer.  As a result, the code that handles placement next to the float scoots the _outer_ table (whose border-box in fact would overlap the float) over to the right.

Can we just put the table margin on the outer table instead of the inner table?
Status: UNCONFIRMED → NEW
Component: Layout: Floats → Layout: Tables
Ever confirmed: true
QA Contact: layout.floats → layout.tables
Summary: CSS float problem → Top margin on table doesn't allow it to clear a previous float

Comment 5

7 years ago
(In reply to comment #4)
> Can we just put the table margin on the outer table instead of the inner table?

Would this fix bug 87277 as well?
Possibly.  I won't pretend to understand the margin-c code.  ;)

Comment 7

6 years ago
(In reply to Boris Zbarsky (:bz) from comment #6)
> Possibly.  I won't pretend to understand the margin-c code.  ;)

Table margins will be put on the outer table frame in bug 659828. This will fix bug 87277. In a try build with bug 659828's latest patch, this issue is fixed as well.

There are similar issues like bug 543070 or the testcase in bug 105520 comment 47, however, those seem to be part of bug 25888 and won't be fixed by bug 659828.
Depends on: 659828
Keywords: testcase
Version: unspecified → Trunk
Created attachment 563569 [details] [diff] [review]
Reftest

This bug is now fixed.  This is the reftest.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #563569 - Flags: review?(roc)
Attachment #563569 - Flags: review?(roc) → review+

Comment 9

6 years ago
Try run for 5f5ead90bfa5 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5f5ead90bfa5
Results (out of 48 total builds):
    success: 46
    warnings: 2
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-5f5ead90bfa5
https://hg.mozilla.org/mozilla-central/rev/d6ce4d56e121
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Keywords: dev-doc-needed
This is marked as needing documentation but I'm not sure what needs to be said. Anyone have a summary of what this change is that's human readable?
(In reply to Eric Shepherd [:sheppy] from comment #11)
> This is marked as needing documentation but I'm not sure what needs to be
> said. Anyone have a summary of what this change is that's human readable?

If you've documented bug 87277, there's nothing more to do here.
This isn't the bug I'm looking for.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.