Last Comment Bug 564002 - Top margin on table doesn't allow it to clear a previous float
: Top margin on table doesn't allow it to clear a previous float
Status: RESOLVED FIXED
: dev-doc-complete, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla10
Assigned To: :Ehsan Akhgari
:
Mentors:
http://silverfalls.orvsd.org/do/calendar
Depends on: 659828
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-05 12:02 PDT by ataylor32
Modified: 2012-01-04 12:53 PST (History)
8 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
reduced testcase (451 bytes, text/html)
2010-05-05 16:24 PDT, j.j.
no flags Details
Reftest (1.83 KB, patch)
2011-09-29 15:23 PDT, :Ehsan Akhgari
roc: review+
Details | Diff | Splinter Review

Description ataylor32 2010-05-05 12:02:27 PDT
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 j.j. 2010-05-05 16:24:20 PDT
Created attachment 443753 [details]
reduced testcase

Behaves different in any other browser
Comment 2 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2010-05-05 17:11:02 PDT
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.
Comment 3 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2010-05-05 17:11:36 PDT
Or perhaps the difference is that other browsers are checking the space below the margin-top of the table?
Comment 4 Boris Zbarsky [:bz] 2010-05-05 21:48:13 PDT
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?
Comment 5 j.j. 2010-05-06 11:18:26 PDT
(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?
Comment 6 Boris Zbarsky [:bz] 2010-05-06 11:20:29 PDT
Possibly.  I won't pretend to understand the margin-c code.  ;)
Comment 7 Daniel.S 2011-08-23 12:19:58 PDT
(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.
Comment 8 :Ehsan Akhgari 2011-09-29 15:23:55 PDT
Created attachment 563569 [details] [diff] [review]
Reftest

This bug is now fixed.  This is the reftest.
Comment 9 Mozilla RelEng Bot 2011-09-29 22:11:35 PDT
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
Comment 11 Eric Shepherd [:sheppy] 2011-12-12 07:40:32 PST
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?
Comment 12 :Ehsan Akhgari 2012-01-04 12:45:20 PST
(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.
Comment 13 Eric Shepherd [:sheppy] 2012-01-04 12:53:34 PST
This isn't the bug I'm looking for.

Note You need to log in before you can comment on or make changes to this bug.