Closed Bug 155955 Opened 22 years ago Closed 16 years ago

[BC] outer half of collapsed table border bleeds out of table box (border-collapse)

Categories

(Core :: Layout: Tables, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: tar, Assigned: bernd_mozilla)

References

(Depends on 2 open bugs)

Details

(Keywords: css2, testcase)

Attachments

(7 files, 2 obsolete files)

From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1a+) Gecko/20020629 BuildID: 2002062904 Using style="float: left" on tag before table tag causes table (with style="margin: 0px 0px 0px 1px" set) to have visible style of "margin-top: -1px, margin-left: -1px" but DOM Inspector says that it has 0px margin all the way. In second row containing div and table with 10px borders it is seen that the elements collide. This bug only appreas when running in strict mode, removing doctype and therefore going to quirks mode displays it correctly. Reproducible: Always Steps to Reproduce: 1. Open testcase 2. See wether boxes are aligned and with 1px spacing Actual Results: table box is not correctly aligned Expected Results: table box is at same height and has 1px margin either side M$ IE 6.0 displays the testcase as expected.
Attached file testcase, HTML CSS (deleted) —
Keywords: testcase
Attached image expected rendering (deleted) —
Given 'border-collapse: collapse' this seems like the correct result. See CSS2 17.6.2.
QA Contact: petersen → moied
Attached file Testcase #2 (deleted) —
It seems the origin of a collapsed border table is set where the upper-left grid lines meet. See: http://www.w3.org/TR/REC-CSS2/tables.html#collapsing-borders I don't know if this is a bug or not. --> HTMLTables.
Assignee: attinasi → karnaze
Severity: normal → minor
Component: Layout → HTMLTables
QA Contact: moied → amar
Summary: strict mode CSS "float: left" for table uses wrong margin-top and margin-left → Origin of a collapsed border table is offset by half a border
What would be the workaround to display floated table in strict mode Mozilla as it is rendered with MSIE? And how is it useful being rendered as it is currently by strict mode Mozilla?
After reading the spec some more, I agree with Tarmo - this is a bug. It breaks the box model, CSS2 8.1 and 9.4.1 for example.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: css2
Attached file Testcase #3 (deleted) —
Note the sentence: "Note that in this model, the width of the table includes half the table border." I still don't think this is a bug.
Yes, I saw that sentence, it is also indicated in the figure directly above. What I don't understand is why this has anything to do with the positioning of the table. Assuming 'margin:0', then the "outer edge" is the "border edge" (8.1). I don't see anything in 17.6.2 that indicates an exception to this rule.
Hmm. I always interpreted that as meaning both outer width and inner width, but I guess it could mean just inner width.
Blocks: 170281
Priority: -- → P4
Target Milestone: --- → Future
There's also this: "Note only half of the two exterior borders are counted in the table width; the other half of these two borders lies in the margin area." If the margin has zero width, how can 5px of border lie in it? The margin would have to be at least 5px wide for a 10px collapsed border. The question is, should the space be added to the margin or should the space collapse with it?
fantasai: thats a really nice sentence, how did I overlook this treasure. We can easily incorporate this in nsTableOuterFrame.cpp. This would yield that the table does not leak outside the outer table frame. I like this. Probably adding the space is the clearer way to go.
Hmmm.. I think that even if we add the space for a border that takes effect along the whole edge (table, row group, col group, col, row), we should collapse any extra due to individual cells. Suppose I had <table> <tr><td>a</td><td>b</td></tr> <tr><td id="c">c</td><td>d</td></tr> <tr><td>e</td><td>f</td></tr> </table> table { margin: auto; border: 2px solid; border-collapse: collapse; width: 80%; } #c { border: 10px solid; } We want the table to appear centered, but if the extra 4px gets added to the left margin, it will be off-center.
OS: Windows 2000 → All
Hardware: PC → All
I would rather not collapse them for cell but let them leak out of the outer table frame.
If you let the borders leak out, they'll overpaint adjacent content. Like in my testcases for 4510: the top border overwrites part of the caption. And, it says "exterior borders", not "exterior row, col, table, colgroup, or row group borders". :) So, you should find the greatest effective border width on that side and the greatest continuous (row/col/row group/col group/table) border width. Add the greatest continuous border width and make sure that the resulting margin is at least as big as the greatest effective border width. ... Suppose that the table border is 10px, but all the cells along the top have border: hidden. Would it make sense to leave room for a border that doesn't exist?
The current model, as I understand it, lets you lay out the first row of a fixed layout table without knowing the border widths of subsequent rows' first and last cells. If you change the model to have the table width go up to the widest border, then your table width can change upon receiving the data for subsequent rows. I don't pretend to understand this stuff though. Did I mention I hate tables?
If you would not hate tables, the css2.1 spec would be implementable and may be self consistent and I would have a person to ask :-). At least the question has been posted to w3c-style: http://lists.w3.org/Archives/Public/www-style/2003Jan/0230.html Did I mention that there was no reply, as usual with table related questions?
Ian - In border-collapse mode, table width is measured from the center of the border, not from the edge. http://www.w3.org/TR/REC-CSS2/tables.html#collapsing-borders The extra width from the border would become part of the margin, so fixed layout would still work. (The margin only affects the table width if the width is auto, and if the width is auto we use auto layout.)
fantasai: yes, but haven't you requested that that be changed?
Attached patch patch (obsolete) (deleted) — Splinter Review
> fantasai: yes, but haven't you requested that that be changed? No, Bernd did. I don't agree with him on that. :) I still think the extra width should be part of the margin--and if you consider table backgrounds (*grin*) you'll see that it only makes sense this way. I should've replied by now, though.
mass reassign to default owner
Assignee: karnaze → table
QA Contact: amar → madhur
Target Milestone: Future → ---
Target Milestone: --- → Future
Blocks: 212036
*** Bug 224492 has been marked as a duplicate of this bug. ***
*** Bug 210175 has been marked as a duplicate of this bug. ***
http://fantasai.inkedblade.net/style/discuss/collapsed-outer-border/ At the 2004 Plenary, the CSS WG decided to leave this detail of border-collapse behavior undefined. I recommend we follow Opera's example and implement what I've decribed in the www-style proposal above.
*** Bug 170281 has been marked as a duplicate of this bug. ***
Assignee: core.layout.tables → fantasai.bugs
QA Contact: madhur → ian
Summary: Origin of a collapsed border table is offset by half a border → [BC] outer half of collapsed table border bleeds out of table box
Attachment #115653 - Attachment is obsolete: true
Blocks: 254035
*** Bug 255029 has been marked as a duplicate of this bug. ***
*** Bug 277117 has been marked as a duplicate of this bug. ***
*** Bug 280207 has been marked as a duplicate of this bug. ***
(In reply to comment #27) > http://fantasai.inkedblade.net/style/discuss/collapsed-outer-border/ > "The solution ... and add it to the table margin." Instead of adding it one could perhaps inflate it instead? (That is, the margin does not change when the border fits inside and is only made wide enough, but not more, when the border does not fit.)
Summary: [BC] outer half of collapsed table border bleeds out of table box → [BC] outer half of collapsed table border bleeds out of table box (border-collapse)
*** Bug 282224 has been marked as a duplicate of this bug. ***
As long as this bugs lacks a real solution due to a lack of concesus on what to do here (or where is the problem now?), shouldn't an intermediary solution fix the different calculations for left/right and top/bottom positioning as demonstrated by https://bugzilla.mozilla.org/attachment.cgi?id=174298 (from dupe https://bugzilla.mozilla.org/show_bug.cgi?id=282224 -- sorry!)? Then at least the behavior is calculatable, and the discussion could go on forever, as much as I care. FYI: Opera does it just like the attachment proposes, and not not as described by comment #27. Version 7.54 and 8.0b1.
The problem is that this bug has lower priority, than http://tinyurl.com/3hw8y. Feel free to fix it, ask me if you need help.
Should'nt the title of the bug be changed to reflect its evolution ? I agree that the current way, people not understanding (or not reading at all) the W3C specs and filing a bug can see there is already one. Yet, the fact that the border bleeds outside of table is no longer considered the bug to correct, but rather "outer half of collapsed table border should inflate/extend/... the table margin to prevent overlap" or something like that. Doing so, the bug may become invisible to people wanting the border to be _in_ the box, but volontary programmers could discover a bug they _do want_ to fix because it better fits their view of the world. _FrnchFrgg_ P.S.: Is it complicated to a newbie in Mozilla's code (but not in coding) to take over this bug and try to write a patch ? Given a hint as to the place where collapsed border's code hides, I could try (and maybe loose, you've been warned ;)
>Is it complicated to a newbie in Mozilla's code (but not in coding) to take over this bug and try to write a patch ? Hmm it would be not fair to say its easy for a newbie however its feasible its not rocket science. The table margin handling is done at http://lxr.mozilla.org/seamonkey/source/layout/tables/nsTableOuterFrame.cpp But the first step is to get a working build (http://www.mozilla.org/build/)
(In reply to comment #37) Shouldn't it be in nsTableFrame ? Because it is its margin that matters, and I would rather have a table border non leaking into it's caption frame, do I ? _FrnchFrgg_
No, its the outer table frame that takes care of its child (the table itself and the caption) margins.
What I meant is : If there exists a function that returns margins of the (inner child) table, and whose result is used by the outer container to a) calculate its caption position & size, and b) exhibit some margins too (I'd say 3 unmodified margins of the table, and one of the caption), I'd rather modify this one. And I would have searched such a function in the inner table class, because it seems to belong to it. Is GetMarginPadding(...) such a function ? And in some other functions OuterReflowChild(...), nearly identical code is used as in GetMarginPadding. The main problem for me is that not knowing exaclty the way you reflow and when you calculate margins, I have to guess, and I may gess wrong, or forgot somewhere... I'll try to go on the dev forums to find help, unless some short explaination can be given here without bloating the bug and taking to much time of the happy few who know. _FrnchFrgg_
Making the margin adjustments in nsTableOuterFrame.cpp would not give the intended effect when there is a caption. The changes must be in nsTableFrame.cpp. I just went looking through the minutes of the csswg meeting. It was suggested that the collapsed border width figure in the calculation of the table's computed border width. I think I need to run a few tests on what Opera's doing before we decide how exactly we're going to fix this bug.
*** Bug 287011 has been marked as a duplicate of this bug. ***
*** Bug 291104 has been marked as a duplicate of this bug. ***
*** Bug 291147 has been marked as a duplicate of this bug. ***
*** Bug 293870 has been marked as a duplicate of this bug. ***
Severity: minor → normal
OS: All → Windows XP
Priority: P4 → --
Target Milestone: Future → ---
OS: Windows XP → All
For the record, the spec changed several times on this. When fixing this bug, make sure to read the new text (section 17.6.2), in particular about how the first row is magical and never spills out, but that subsequent rows may spill out if they have wider borders: # UAs must compute an initial left and right border width for the table by # examining the first and last cells in the first row of the table. The left # border width of the table is half of the first cell's collapsed left border, # and the right border width of the table is half of the last cell's collapsed # right border. If subsequent rows have larger collapsed left and right borders, # then any excess spills into the margin area of the table.
Assignee: fantasai.bugs → nobody
QA Contact: ian → layout.tables
*** Bug 346533 has been marked as a duplicate of this bug. ***
*** Bug 347130 has been marked as a duplicate of this bug. ***
Note that when we implement what the spec says, I think we can do it in quirks mode too and remove our implementation of a similar idea (but somewhat different rules) in quirks mode.
What is the reason for calculating the width and height of the table using half of the border width/height? Wouldn't it be more logical to draw the left/right and top/bottom borders of the table inside the grid lines? (http://www.w3.org/TR/REC-CSS2/tables.html#collapsing-borders) If you open the following page (http://dev.l-c-n.com/tables/borderCollapseTest.html) and use Firebug (or similar) to change the border-color of the divs to green (#0C0), you can see how ridiculous it looks at the moment with no table margins (compared to the 'border-collapse off' rendering). I don't see the point/meaning of auto-adding margin just to accommodate the border. It doesn't make sense placing part of the border into the margin, IMHO. Aren't margins supposed to be empty, and used for spacing between elements? I don't really know much about this subject, so please keep that in mind when reading the above.
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1+
fantasai: we have three functions at http://mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableFrame.h#291 in a new CSS 2.1 compliant world what would the tree functions return nsMargin GetOuterBCBorder() const; the new table border width? as defined by the width on the first and last cell on the first row together with the top border and bottom border on the first cell in the first and last row? nsMargin GetIncludedOuterBCBorder() const; the new table border width? nsMargin GetExcludedOuterBCBorder() const; 0?
"That is actually what the spec says. http://www.w3.org/TR/CSS21/tables.html#collapsing-borders" >>> philippe: But that part of the specification only states how the collapsed borders must be rendered according to the internal table grid, not how the whole border must be aligned with the container box, Right? In my opinion that text "only" means that if you apply a border of 4 px, 2 px of it will be put out of the grid line and 2 px will be put inside. That doesn't mean that the alignment of the table has to be referred to the mentioned grid line. There's no line which says something like that. Am I wrong? Indeed, doing that would contradict the "sense" of the CSS borders, as they take a room and they "push" any surrounding element (except if you use rules to alter that, obviously). The present implementation converts the border into a weird mixture: The inner half behaves like a border, whereas the external half acts like an outline.
Based on the semantics of these functions, nsMargin GetOuterBCBorder() const; would return everything that spills out, as I imagine it does today nsMargin GetIncludedOuterBCBorder() const; would be: * on the left, 1/2 the collapsed left border width on the first cell in the first row * on the right, 1/2 the collapsed right border width on the last cell in the first row * on the top, 1/2 the widest collapsed top border segment on the first row * on the bottom, 1/2 the widest collapsed bottom border segment on the last row nsMargin GetExcludedOuterBCBorder() const; would return GetOuterBCBorder() - GetIncludedOuterBCBorder() GetIncludedOuterBCBorder() would be used for width/margin calculations and would be included in table background painting. GetExcludedOuterBCBorder() would be used only for calculating overflow. Personally, I don't like these rules. I think - the table's own border should be used for GetIncludedOuterBCBorder() - that GetOuterBCBorder() should be calculated as 1/2 max collapsed border on all sides, falling to the 2.1 rules only for fixed tables - that GetExcludedOuterBCBorder() should collapse with the table's margins but that's not what 2.1 says...
fantasai: thanks, is there a reason why the left/right and top/bottom borders are treated differently? >but that's not what 2.1 says... you should talk to the editors ;-)
The goal there was to avoid two passes over the table.
My 3.0.3 Firefox browser was updated to 3.0.4 and the left border of my home page disappeared. It's a xhtml 1.0 strict validated page. It used to work perfectly in 3.0.3. Here's my home page: http://www.susceptor.be
Hi all. This bug really strains. Somebody knows when will correct?
Attached patch patch (obsolete) (deleted) — Splinter Review
This does roughly what is required, it lacks testing.
re comment 64 Jan this is a old bug it is certainly unrelated to what you see.
Attached patch patch (deleted) — Splinter Review
Assignee: nobody → bernd_mozilla
Attachment #350600 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #355133 - Flags: review?(fantasai.bugs)
Comment on attachment 355133 [details] [diff] [review] patch Please update the comments in nsTableFrame.h for the functions you are changing. + <tr><td style="border:solid 4px red; height:30px">cell 1</td></tr> Use a color other than red, since the presence of red is supposed to indicate failure. + if (propData) { + border.top += BC_BORDER_TOP_HALF_COORD(p2t, propData->mTopBorderWidth); This should be =, not +=. - nsMargin offset(0,0,0,0); + nsMargin offset(0,0,0,0); Stray whitespace deletion. r=fantasai otherwise
Attachment #355133 - Flags: review?(fantasai.bugs) → review+
Attached patch revised patch (deleted) — Splinter Review
Attachment #357485 - Flags: superreview?(roc)
Attachment #357485 - Flags: superreview?(roc) → superreview+
Should this be fixed in the following build? Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090208 Minefield/3.2a1pre (.NET CLR 3.5.30729) ID:20090208181212 The test case is still not proper if it's supposed to be. ~B
If you do not see this in a build from 2009-02-09 please reopen the bug, there are several reftests that run on tinderbox that rely on this changed behaviour.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Still not fixed in: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090209 Minefield/3.2a1pre (.NET CLR 3.5.30729) ID:20090209020258 ~B
Bryan, what exactly is not fixed? You mean the first testcase in this bug?
Martijn, Yes, the first testcase. Maybe I've misunderstood this bug. ~B
Blocks: 410621
Did this patch actually land somewhere ? I don't see where/when/by whom it was checked into either trunk or branch. Perhaps 'fixed' by another bug that was not referenced ?
Flags: in-testsuite? → in-testsuite+
Attached file test case (deleted) —
OK, seems to be fixed in 3.6a1pre! Will this patch also be applied to 3.5?
Not likely.
Has this bug fixed Bug 489279 ?
Issue no longer shows up after install of 3.6a1! :)
Depends on: 484258
Depends on: 484260
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: