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

RESOLVED FIXED

Status

()

Core
Layout: Tables
RESOLVED FIXED
15 years ago
3 years ago

People

(Reporter: Tarmo Järvalt, Assigned: Bernd)

Tracking

(Depends on: 2 bugs, {css2, testcase})

Trunk
css2, testcase
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 2 obsolete attachments)

(Reporter)

Description

15 years ago
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.
(Reporter)

Comment 1

15 years ago
Created attachment 90342 [details]
testcase, HTML CSS
(Reporter)

Updated

15 years ago
Keywords: testcase
(Reporter)

Comment 2

15 years ago
Created attachment 90343 [details]
expected rendering
Given 'border-collapse: collapse' this seems like the correct result.  See CSS2
17.6.2.

Updated

15 years ago
QA Contact: petersen → moied
Created attachment 92697 [details]
Testcase #2
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
(Reporter)

Comment 6

15 years ago
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
Created attachment 94882 [details]
Testcase #3
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

Updated

15 years ago
Priority: -- → P4

Updated

15 years ago
Target Milestone: --- → Future

Comment 12

15 years ago
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?
(Assignee)

Comment 13

15 years ago
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.

Comment 14

15 years ago
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
(Assignee)

Comment 15

15 years ago
I would rather not collapse them for cell but let them leak out of the outer
table frame.

Comment 16

15 years ago
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?
(Assignee)

Comment 18

15 years ago
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?

Comment 19

15 years ago
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?
(Assignee)

Comment 21

15 years ago
Created attachment 115653 [details] [diff] [review]
patch

Comment 22

15 years ago
> 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.

Comment 23

15 years ago
mass reassign to default owner
Assignee: karnaze → table
QA Contact: amar → madhur
Target Milestone: Future → ---

Updated

15 years ago
Target Milestone: --- → Future
See also bug 170281 (testcase: attachment 100223 [details])
Blocks: 212036
*** Bug 224492 has been marked as a duplicate of this bug. ***

Comment 26

14 years ago
*** Bug 210175 has been marked as a duplicate of this bug. ***

Comment 27

14 years ago
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.

Comment 28

14 years ago
*** Bug 170281 has been marked as a duplicate of this bug. ***

Updated

14 years ago
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

Updated

14 years ago
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)
(Assignee)

Comment 33

13 years ago
*** Bug 282224 has been marked as a duplicate of this bug. ***

Comment 34

13 years ago
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.
(Assignee)

Comment 35

13 years ago
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 ;)
(Assignee)

Comment 37

13 years ago
>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_
(Assignee)

Comment 39

13 years ago
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_

Comment 41

13 years ago
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.
(Assignee)

Comment 42

13 years ago
*** Bug 287011 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 43

13 years ago
*** Bug 291104 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 44

13 years ago
*** Bug 291147 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 45

12 years ago
*** Bug 293870 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Severity: minor → normal
OS: All → Windows XP
Priority: P4 → --
Target Milestone: Future → ---
(Assignee)

Updated

12 years ago
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. ***
Duplicate of this bug: 369730
Duplicate of this bug: 319350
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.
Duplicate of this bug: 390617
Duplicate of this bug: 435019

Updated

9 years ago
Duplicate of this bug: 448077

Comment 55

9 years ago
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+
Duplicate of this bug: 322236
Duplicate of this bug: 461486
(Assignee)

Comment 58

9 years ago
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?

Updated

9 years ago
Duplicate of this bug: 461699

Comment 60

9 years ago
"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.

Comment 61

9 years ago
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...
(Assignee)

Comment 62

9 years ago
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 ;-)

Comment 63

9 years ago
The goal there was to avoid two passes over the table.

Comment 64

9 years ago
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

Comment 65

9 years ago
Hi all. This bug really strains. Somebody knows when will correct?
(Assignee)

Comment 66

9 years ago
Created attachment 350600 [details] [diff] [review]
patch 

This does roughly what is required, it lacks testing.
(Assignee)

Comment 67

9 years ago
re comment 64 Jan this is a old bug it is certainly unrelated to what you see.
(Assignee)

Comment 68

9 years ago
Created attachment 355133 [details] [diff] [review]
patch
Assignee: nobody → bernd_mozilla
Attachment #350600 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #355133 - Flags: review?(fantasai.bugs)

Comment 69

9 years ago
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+
(Assignee)

Comment 70

9 years ago
Created attachment 357485 [details] [diff] [review]
revised patch
Attachment #357485 - Flags: superreview?(roc)
Attachment #357485 - Flags: superreview?(roc) → superreview+

Comment 71

9 years ago
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
(Assignee)

Comment 72

9 years ago
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
Last Resolved: 9 years ago
Flags: in-testsuite?
Resolution: --- → FIXED

Comment 73

9 years ago
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?

Comment 75

9 years ago
Martijn,

Yes, the first testcase.  Maybe I've misunderstood this bug.

~B

Updated

9 years ago
Blocks: 410621
Duplicate of this bug: 482226
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 ?
It's fixed on trunk AFAICT:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/
as of 2009-02-08 it seems:
http://hg.mozilla.org/mozilla-central/rev/fc7431820ea3
(Assignee)

Updated

9 years ago
Flags: in-testsuite? → in-testsuite+

Updated

9 years ago
Duplicate of this bug: 470977

Comment 80

9 years ago
Created attachment 371352 [details]
test case

Comment 81

9 years ago
OK, seems to be fixed in 3.6a1pre! Will this patch also be applied to 3.5?
Not likely.

Updated

9 years ago
Duplicate of this bug: 333643
Has this bug fixed Bug 489279 ?

Comment 85

8 years ago
Issue no longer shows up after install of 3.6a1! :)
(Assignee)

Updated

8 years ago
Duplicate of this bug: 538658
Depends on: 484258
Depends on: 484260
You need to log in before you can comment on or make changes to this bug.