68061 - without wrapping tr in tbody, table does not render correctly in XHTML

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect, P2)

Description

[Chris Petersen]

Build: 2001020608
Platforms: All
Expected Result: Background color should displayed in table
What I got: Background color is only partially displayed. Character data in the TD
is opaque and overlaps background color.

Steps to reproduce

1) Open either test case:

http://slip/projects/marvin/xhtml/transitional/table_bgcolor_rgb.xml


http://slip/projects/marvin/xhtml/transitional/tr_bgcolor_rgb.xml



2) In the table_bgcolor_rgb.xml
, the bgcolor attribute appears in the TABLE element.

 In the tr_bgcolor_rgb.xml
, the bgcolor attribute appears in the TR element.

3) Notice that the background color specified is not completely displayed. The
TD 's contents is opaque and covers the background color.

Comment 1

[Chris Petersen]

Found out more info about the problem. Each test case has a bgcolor attribute on
the BODY element. A value of "#FFFFFF has been assigned.Removing the bgcolor
attribute from body will allow the background color of the TR/TABLE will display
correctly.

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I believe it is possible to work around this by using CSS stylesheet.

This bug has been marked "future" because the original netscape engineer
working on this is over-burdened. If you feel this is an error, that you or
another known resource will be working on this bug,or if it blocks your work
in some way -- please attach your concern to the bug for reconsideration.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Heikki: could please reevaluate this since I think this is what's causing bug 
53944. If it is then bgcolor on tables will be broken for xslt generated pages

Chris: could you please attach the testcases here so that non-netscape people 
can test them, thanks

Comment 4

[Chris Petersen]

Here are the same tests found at mozilla.org:

http://mozilla.org/quality/browser/standards/xhtml/transitional/
table_bgcolor_rgb.xml

http://mozilla.org/quality/browser/standards/xhtml/transitional/
tr_bgcolor_rgb.xml

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Jonas, would you have the cycles to track this down? It is most likely just a QI
to nsIHTMLDocument somewhere that we could get rid of, or extend it for
nsIXMLDocument as well. Tracking it down is the hard part, and I don't know when
I will have the cycles to work with this.

I don't know a good place to place breakpoints to start with since I have not
looked at the code, but here is one suggestion: place a breakpoint in
nsXMLContentSink::OpenContainer and step in to the code that creates the HTML
content object that has the bgcolor attribute. This way you should be able to
find out what class might be the culprit. Then, place breakpoints in the methods
of that class that might have something to do with this to try and track down
where it happens. The bug might be in the content class, or frame class, or a
combination of both or with bad luck it might be somewhere else. I am pretty
sure the content sink has nothing to do with this, but you could see if the HTML
content sink has some special handling for bgcolor attribute.

You could also ask the table people (karnaze?) what would be a good place to
look at.

Comment 6

[Markus Hübner]

Since this does considerably affect rendering of XML/XSL changing severity to
"major".
Possible to get this into 0.9.4 ?

Comment 7

[Peter Van der Beken [:peterv]]

The problem is that for XHTML we're not creating the (optional) TBODY element.
For HTML, this is handled by the parser, which does various fix-up for cases
like this. In this case, it inserts a TBODY element between the TABLE element
and the first TR element. Not sure whether we should handle this in the sink our
in the stylesheet (html.css?).
See http://www.w3.org/TR/html401/struct/tables.html#h-11.2.3. The relevant bit
is "The TBODY start tag is always required except when the table contains only
one table body and no table head or foot sections."
Lowering severity, there's a workaround: wrap your rows in a TBODY.

Comment 8

[Peter Van der Beken [:peterv]]

Attachment: table_bgcolor_rgb.xml, fixed up with tbody's

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Just FYI: XHTML should not create a tbody element in the DOM if it is missing 
from the markup. This is one of the differences between XHTML and HTML. See 
C.11 in http://www.w3.org/TR/html/#guidelines

Comment 10

[Marc Attinasi]

CC karnaze: he says there will be some problems in tables if there is no TBODY...

Comment 11

[Bernd]

see bug 30378 for an extended discussion of the tbody problem

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Oh, I even noticed that in the xhtml DTD <tr> is a valid child of <table> so we
should defenetly be able to deal with that (at least in xhtml)

Comment 13

[Peter Van der Beken [:peterv]]

Jst, you're right. So the problem is a table layout issue. See also bug 86664
which is very similar, not sure if it's a dupe though.

Comment 14

[Peter Van der Beken [:peterv]]

er, I meant Jonas ;)

Comment 15

[karnaze (gone)]

I can't get to the url and the test case works around the problem. Removing the
<tbody>s from the test case doesn't exhibit the problem. Need test cases.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Both tables should look equal appart from contained text

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: borders are affected. Both tables should be equal

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

*** Bug 86664 has been marked as a duplicate of this bug. ***

Comment 19

[basic]

my guess that moz should be just creating an anonymous table-group object when
there is no tbody

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

depends on what you mean by "anonymous table-group object". We can't add a 
tbody node in the DOM since that would break the DOM Core spec. We could 
however add a table-group frame, but i'm not sure if that would help, or if 
there even is such a frame

Comment 21

[basic]

I was thinking more in terms of this:
http://www.w3.org/TR/1998/REC-CSS2-19980512/tables.html#anonymous-boxes
http://www.w3.org/Style/css2-updates/REC-CSS2-19980512-errata.html#x25

I should have said table-row-group and not table-group

it seems that it says:

<quote>
* If the parent P of a 'table-row' element T is not a 'table', 'inline-table',
or 'table-row-group' element, an object corresponding to a 'table' element will
be generated between P and T.
</quote>

but it also says

<quote>
* If a child T of a 'table' element (or 'inline-table') P is not a
'table-row-group', 'table-header-group', 'table-footer-group', or 'table-row'
element, an object corresponding to a 'table-row' element will be generated
between P and T.
</quote>

I'm not sure if it would help this bug, but that is how moz should be behaving.

Comment 22

[basic]

doh! Forget what I said. I'm now wondering if we should suggest to the CSS WD to
amend it to make table-row not a child of table.

Comment 23

[Kevin McCluskey (gone)]

Moving to Moz 1.1. Engineers are overloaded with higher priority bugs.

Comment 24

[Markus Hübner]

As this affects all rendering of XML/XSL I do recommend this for 1.0

Comment 25

[basic]

changing summary from:
Table/TR bgcolor attribute is not render correctly in XHTML
to:
without wrapping tr in tbody, table does not render correctly in XHTML

I would suggest that whenever a table-row frame is found to be a child of a
table frame, moz create a table-group frame. Though it doesn't follow the CSS2
spec but it does makes things easier and simpler, and I don't think there will
be any visible differences.

Comment 26

[karnaze (gone)]

mass reassign to default owner

Comment 27

[Josh Birnbaum]

*** Bug 205481 has been marked as a duplicate of this bug. ***

Comment 28

[Boris Zbarsky [:bzbarsky]]

Bug 211636 explains a good bit of the problem in this bug and the bugs it blocks.

Comment 29

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Taking.

Comment 31

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 127601 [details] [diff] [review]
patch

Please add a comment that calling GetTable like:

nsIConten* tableContent;
GetTable(&tableContent);

(with a pointer to an uninitialized nsIContent*) will most likely crash, ok? 
It's probably safe to assume that won't happen since this is a protected
method, but....

Comment 32

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why?  The out param is initialized to null at the beginning of |GetTable|.

Comment 33

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2003-07-12 15:13 -0700.

Comment 34

[Bernd]

*** Bug 205705 has been marked as a duplicate of this bug. ***

Comment 35

[Asa Dotzler [:asa]]

Comment on attachment 127601 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to 1.4 branch.

Comment 36

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_4_BRANCH, 2003-08-08 02:17 -0700.

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

*** Bug 193529 has been marked as a duplicate of this bug. ***

Comment 38

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

*** Bug 15799 has been marked as a duplicate of this bug. ***

Comment 39

[Boris Zbarsky [:bzbarsky]]

Pushed the tests as http://hg.mozilla.org/mozilla-central/rev/3e0b11910081