From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows 98) BuildID: 2000061311 When bgcolor is used on a table, Mozilla does not display the background color on the entire table. Instead, wherever there are spaces between table cells the background table color is missing. Reproducible: Always Steps to Reproduce: 1. Make a table in a html page 2. Insert the bgcolor="#000000" tag into the table tag like so: <table bgcolor="#000000"> Actual Results: The individual cells of the table fill with the specified bgcolor (background color) however, the spaces in between cells do not fill with the specified color. Thus, the entire table does not fill with the specified bgcolor. Expected Results: The entire table (including space between and around cells should fill with the specified table background color. See Internet Explorer as an example.

Probably duplicate of bug 4510.

No, this is different. It deals with HTML's 'bgcolor', which, AFAICS, should only affect table cells, not the cellspacing; Nav4 and Mozilla display this correctly. http://www.w3.org/TR/html4/present/graphics.html#adef-bgcolor No matter how you interpret "canvas", MSIE is displaying things wrong: Either you put the backgrounds on the cells, or you put the backgrounds on the entire element it is specified on. MSIE is not doing this for table row backgrounds. However, painting the background on table cells is *explicitly specified in the definition*, not ambiguously in the prose, and Navigator has been doing that for years. FYI: - Nav3 & Nav4 display backgrounds only on table cells. - Opera, IE3, & IE5 also display backgrounds specified on <table> in the cellspacing, but don't do this for <tr>. - Amaya paints the background on the entire <table> and the entire <tr>. I'd like to resolve this as invalid, but in light of the behavior of current browsers, I leave the decision to others.

Mozilla is rendering the backgrounds on cellspacing in the testcase.. but not where I extracted it from. The Nav4 behavior is caused by Quirks mode.

Although I can only see the HTML4 specification as allowing bgcolor to affect table cells, MSIE implemented this in version 2.0 (before the HTML4 spec added 'bgcolor' to the list of valid attributes for <table>). *If MSIE's definition is to be followed for Quirks, which it should if Gecko is displaying this differently in Quirks at all, then 'bgcolor' on <table> should color the whole table, including cellspacing, but 'bgcolor' on <tr> should not affect cellspacing.* Confirming bug to that end. Sorry for the mess.

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

The spec does not say "if applied to a table element, it should only affect cells". I don't know what they meant by "body and table cells", but they go on to say it sets the background color for "tables". A table is a single entity and not a collection of independent cells. Rendering bgcolor in cells only leaves developers with having to do: <table bgcolor="#000000"> <table cols ="2" rows="2"> ... Nesting tables everytime one wants the appearance of a *single* table with a solid background will irritate developers a lot. Where the spec is ambiguous, err on the side of what developers will want. That is what the specs are ostensibly for anyway.

To clarify: This bug was confirmed to change the Quirks handling to imitate Microsoft's, which /does/ fill cellspacing on the table (just not the <tr>). So the spec wording here is a non-issue.

Ooohhhh. I should have looked at the testcase. This is bad. I had assumed that we were talking about border=0 cellspacing=>0. There is no way, whether quirks or not, that bgcolor of a table should be rendered in the cell spacing *when the border is set to a positive integer*. When the border=0 it should fill in the cellspacing since there is no border to cover it up, but not when there is a border defined.

> When the border=0 it should fill in the cellspacing since there is no border > to cover it up, but not when there is a border defined. That's inconsistent. Background color rules should not depend on the border property. And if the author wants such an effect, they just increase the cellpadding and set the cellspacing to zero, which has the advantage of working in Netscape 3.x & 4.x, too.

Not inconsistent at all. The very fact that you can assign a color to a table border implies that they overlay the background (otherwise what's the point?). Another way to think of it is that a background is a background after all, and table borders are not part of the background.

I'd just like to say, I think the BGcolor on a table should effect the cellspacing. I can't think of a single reason why it should not, and I can think of serveral reasons why it should. this "bug" gets my vote:)

Because a border is not transparent might be one reason???

The actual issue here is (to clarify my origional post) the cellspacing between each of the cells, not the table borders. Looking at it from a pure html perspective - the bgcolor in the table tag should apply to the entire table, not pieces of a table - If you want to apply colors just to the cells of a table and not the cellspacing, you put the bgcolor in each of the td tags (cells) to be colored. From a pure html purpective (and only from that perspective), that is how it naturally <i>should</i> work. Properties of a tag should apply to the entire tag, not pieces of if. From this perspective, it could also bring up the issue of coloring table borders: http://bugzilla.mozilla.org/show_bug.cgi?id=46265 From a somewhat practical perspective, should cellspacing not contain the entire table's color? Perhaps.... The only practical example of why you would want it to defer from the exepected <i>norm</i> is in a 3D border for example: bgColor=3D"#000080" border=3D"0". In this instance it might seem odd to have the bgcolor fill the cellspacing. I've may have brought up more questions that I've clarified. From a pure html standpoint, properies applied to a tag like the table should apply to the whole tag, and not pieces of it. While from a more pratical application standpoint, you could go either way, with the exception maybe of 3D borders. The question to answer is, are cells to be treated as divisions within a table? or are cells to be treated as entities to themselves? The way I see it, tables cover all of the cells within it. Any property applied to the table affects the entire table and is separate from the property of individual cells.

That's like saying that a page's background should color every element since they are just parts of the body. It's silly, and so is coloring borders the color of the background in a table. BTW there is no "3D" attribute of table borders in the W3C specification that I am aware of.

> Because a border is not transparent might be one reason??? Why'd the borders come in? This is purely a background bug! Re: kevinar's post Looking at it from a pure HTML standpoint, 'bgcolor' shouldn't exist at all. Looking at a strict interpretation of the HTML spec, bgcolor doesn't affect cellspacing. (I'll send you a short proof if you want it.) But since its original inventor (MS) decided that it does, and most people expect that it will, 'bgcolor' should, at least in Quirks mode, color the entire inner table frame. I hope that last sentence summarizes this report's intent? Correct me if I'm wrong.

Re: Jerry Baker's Comments Actually, a page's background does affect the whole body of the page. When you insert tables and other things, the background still remains behind those elements. I believe I understand what you mean, and I think you misunderstood my post. I don't mean that a td or tr should be overwritten by the bgcolor in the table tag, but that things should work in the manner like in the body. bgcolor does affect the whole body as bgcolor (in my opinion) should affect the whole table unless things are inserted like cells (td) with different colors or another table, etc... into the table. As for coloring the borders, that was a whole other issue in another "bug" report. I don't mean the table bgcolor should change the boardercolor; of course a table's bgcolor should not affect the boardercolor. As for the 3D attribute, that was just a forsight for your consideration allowing for the possibility that it may be added to W3C in the future. Re: fantasai@escape.com I guess you'll have to explain a little about what you mean by bgcolor shouldn't exist at all. If you want to know what I meant by a "pure html standpoint", I was not speaking of WC3 standards. In my view (and some will not agree) of html, a property applied to a tag should ecompass the entire scope of that tag. As an example to explain what I mean, bgcolor and other things applied to the body apply to the whole body; properties applied to a layer affect that whole layer. This does not mean that a table or some other tag should take on the properties of the body or layer it's in though.

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

What is the status of this bug...is it being fixed? I don't understand why some people think this isn't a bug. Look at it this way...from a side view as layers: v---v-----cellpadding cells: --- --- --- enclosed by table: --------------- enclosed by body: ----------------- cellpadding, according the the w3c spec is the spacing BETWEEN cells. It makes no sense that the spacing between cells should be rendered with the body background when they are contained within a table (unless, of course, a table background has not been specified.) As others have stated, doing it the current way forces developers to jump through hoops to get a desired background other than the body background.

er...references to cellpadding should be replaced with cellspacing in my preceding post :/

Corey Puffalt, If I understand your point correctly, that is the same argument I was trying to get accross. I created an example of one way to look at how this thind may work. Created an attachment (id=19723) <a href="http://bugzilla.mozilla.org/showattachment.cgi? attach_id=19723">http://bugzilla.mozilla.org/showattachment.cgi? attach_id=19723</a>

> I guess you'll have to explain a little about what you mean by bgcolor > shouldn't exist at all. HTML was designed to be a structural, not a presentational, language. > What is the status of this bug...is it being fixed? The status of this bug is New--that is, confirmed, but not assigned. > As others have stated, doing it the current way forces developers to > jump through hoops to get a desired background other than the body > background. Which is why this bug was filed, and why I confirmed it--to have the bgcolor cover the entire table frame. Kelvinar, I'm not entirely sure I understand your attachment correctly. Would the code for the last example be <table bgcolor="lime"> <tr> <td>..</td> <td bgcolor="red">..</td> </tr> <tr bgcolor="red"> <td>..</td> <td>..</td> </tr> </table> ? Or would the second row take it's color from bgcolor on its two <td>s?

fantasai@escape.com, At first I would have said the bgcolor would be on the td's or tr's, but then I thought if it was on the tr would that mean the red would extend across the whole bottom row (including the spacing between cells) or just in all of the individual cells? According to the layering in my example the tr probably wouldn't affect spacing between cells. But, I don't know, what do you think? Man this is gettin' complicated :)

As bgcolor on <tr> doesn't affect cellspacing in any major browser, it shouldn't do so in Mozilla's Quirks mode, since Quirks tries to emulate what authors expect. The behavior in Standard layout mode is, IMO, debatable. But that's outside the scope of this bug report. >:) BTW, sorry for misspelling your name. m(_)m

I think I can fix this. The code is in there already for Standard layout; only a few switches need adjustment. However, I am unable to build Mozilla. Is anyone willing to try building with patched source?

fantasai, please attach the patch asap.

The attachment also removes Quirks behavior from background styles applied to row groups and column groups. (see bug 4510) Please check the closing braces on nsTableRowFrame.cpp and nsTableRowGroupFrame.cpp before compiling; I edited the diff file directly a bit, as it didn't seem to reflect the changes properly. (Why? I don't know. I've never done a diff before.) The affected source is in the Paint function.

nsTableRowFrame.cpp should not have been changed.

The 1st attachment invokes standard mode and paints the background of the table on the entire table, then the background of the rows on top of that, then the backgrounds of the cells on top of that. I guess this bug is not concerned with this behavior. The 2nd attachment looks identical to Nav 4.7, because it is a quirk mode page, and that is the way it is supposed to work. If standard behavior is desired, then use a dtd that invokes it. So, why should we take this patch (which needs to be tested with the regression tests) and change quirks behavior to standard behavior?

This bug was opened to change Quirks behavior so that <table bgcolor="x"> paints the entire table frame. - This is the behavior of all versions of MSIE and Opera. There is no need for a Quirk that is peculiar only to Navigator and on which few, if any, pages rely. - Most authors prefer this behavior. The patch also enables Standard rendering for background styles applied to table row groups, and column groups. These are unsupported by Navigator; they take Quirks rendering in MSIE. (As it is not necessary to fixing this bug, I will remove that part of the patch should you so choose.) However, Quirks behavior is _retained_ for table rows. This is the behavior of Navigator, MSIE, and Opera, and is thus the expected behavior.

below follows a list of regression test's that fail due the proposed patch. I support karnaze's view. I will not manual review all these testcases. I think this bug should be held open in order to prevent dupes of the issue. I would recommend to future the bug because for me it is a high risk -- low return issue. With that big number of broken regression tests it seems to me that it is very likely (near to 100% probability) that major sites will be broken. regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\core\verify\col_span.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\core\verify\col_widths_fix_autoFix.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\core\verify\col_widths_fix_fix.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\core\verify\conflicts.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\core\verify\misc.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\core\verify\table_widths.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\core\verify\table_widths.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\viewer_tests\verify\test4.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug1055-1.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug1055-2.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug1067-2.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug14929.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug1302.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug1430.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug14323.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug14929.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug16252.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug17548.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug17587.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug1800.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug18359.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug22122.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug23847.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug27038-2.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug27993-1.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug28928.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug2947.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug30332-1.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug33137.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug5188.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug5538.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug625.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug6304.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug7112-1.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug727.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug9123-1.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\bugs\verify\bug9879.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\other\verify\empty_cells.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\other\verify\wa_table_thtd_rowspan.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\other\verify\wa_table_tr_align.rgd failed regression test c:\moz_sour\mozilla\mozilla\layout\html\tests\table\other\verify\wf_table_index.rgd failed

> I will not manual review all these testcases. I would not expect you to. > With that big number of broken regression tests Most of them shouldn't have failed; this fix should only affect backgrounds, and none of those bugs deal with backgrounds (although several use backgrounds to illustrate other effects). I recognize, however that there may be a problem with the patch. > it seems to me that it is very likely (near to 100% probability) that major > sites will be broken. The intent is to emulate MSIE, whose behavior is very /unlikely/ to break major sites. I can build Mozilla now, and I have run the regression tests. However, I did not get the same results you did. (Only one test failed and only the bug tests ran--no core, etc.) Are there any options I should be enabling? I'm running everything in default configuration on Linux.

Table regression tests mean all tests in all directories listed in rtest.bat in the layout/html/tests/table. Especially core and marvin are necessary. Every bbox mismatch is a failure. Usually I ran the regression tests three times 1. baseline without patch, 2. verify without patch in order to see that the regression tests do not produce false warnings, 3. verify with the patch. I dump the output of the tests into results file (with tee), in order to run grep over it ('rgd failed' works great)afterwards. If a regression tests fail, I separate these tests into a special directory, and try to separate the table that causes the failure, this is sometimes very cumbersome, for instance in core sometimes more than 10 tables are collected into a single test. For me this bug is an RFE, which should be either futured or marked as wontfix. As buster stated in a different bug: We have bigger fish to fry.

Thanks for the tips~ Compiled source pulled on 2000-12-11, on Linux. All tests pass. I ran the tests twice before and twice after applying changes and checked all the non-style data mismatches against the other verification results; everything seems accounted for. (Color & transparency were supposed to change.) Is there anything else I should do? > As buster stated in a different bug: We have bigger fish to fry. Well, I can't fry any fish much bigger than this. :)

Can someone please tell me if I'm understanding the discussion/consensus properly? table bgcolor: {standards: paint cellspacing} {quirks: paint cellspacing} tr bgcolor: {standards: paint cellspacing} {quirks: don't paint cellspacing} I believe this standards rendering is correct because AFAICS it is the only way to control the color of the cellspacing. To use CSS to set all cells bgcolor without painting the cellspacing, all you have to do is td {background-color: #ffc} or whatever. Or if you want an alternative to the tr bgcolor: <tr class="odd"><td>... <tr class="even"><td>... <tr class="odd"><td>... and then do CSS like: tr.odd td {background-color: #080} tr.even td {background-color: #cfc} You might have to do something fancy, like tr.odd > td, if you have nested tables, but the problem is MSIE doesn't yet support child selectors. The point of this comment is to try to summarize and clarify the discussion so far, and hopefully I haven't misunderstood anything.

Would it be possible to get this fixed for mozilla0.9?

QA contact update

I agree that we should try to get this in. See my comments on bug 4510. Just because regression tests show changes doesn't mean there are regressions. The change made a chance to the style contexts and a compensating change to painting of backgrounds. If we're moving closer to MSIE behavior it's more likely that we're going to fix sites than break them.

I don't have a problem with someone checking in this patch (since it just appears to change quirks mode into standard mode), if he/she will also manually review the regression test pages that Bernd cites. My fear is that since most of our tests are in quirks mode (shame on us) we may be exposing a standard mode bug to a wider number of pages. I will personally not be able to check this in for quite a while due the PDT approval process within Netscape. I'm marking this m1.0 just to get it off the untriaged radar.

I will do the necessary work to get this in, but this will be after 0.9.2 as I have already three other patches that should go into the tree....

I just went through my regression test results for those tests Bernd cited: apparently, some of them did not run--there was no mention of them at all. But everything that did run (which was most of them) passed, and the only mismatches I got were in table background, background transparency, and color (see bug 46480). I'm afraid I won't be much help in manual reviewing those testcases, since they wouldn't fail for me. (Well, one did fail, but it passed the second time.) However, if Bernd wants to send me the verification output (full text before and after), I'll go over that.

Chris wrote: My fear is that since most of our tests are in quirks mode (shame on us) we may be exposing a standard mode bug to a wider number of pages. The good news is: you do not need fear it anymore, the bad one is, it is worse than I imagined. If you look at attachment 38472 [details], it becomes clear for me, before this patch can go in, we need to file the bugs about background color in standard mode and make these bugs to block this bug, once the things are less ugly it should be show time for this bug.

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

> we need to file the bugs about background color in standard mode and make > these bugs to block this bug, once the things are less ugly it should be show > time for this bug. Few authors use or format HTML table columns or row groups, and table rows are not affected by this patch.

fantasai: Could you comment, why we should exclude the rows and include the row groups, I would prefer to get rid of the whole quirk stuff, that seems to me more consistent. Could you also comment about the attachment 38472 [details], is this the expected rendering?

As I said in bug 4510: NavQuirks behavior should be kept for <tr bgcolor="x">, though, since all versions of MSIE also render it that way. (And Opera, too.) As David Baron stated, the purpose of Quirks mode is to emulate Quirks that page authors depend on and not those they don't. They generally don't use table rows groups or columns, and Nav4 didn't even support those. (If you look at the comments in nsHTMLStyleSheet.cpp, the main reason for having quirky behavior for table row groups was for simplicity of implementation.) However, setting backgrounds on table rows is common practice, and it behaves in a quirky manner in all the major browsers since MSIE introduced the capability in version 2.0. Therefore we need to emulate that behavior. >Could you also comment about the attachment 38472 [details], is this the expected rendering? Your testcase uses two green backgrounds on top of each other, so I can't tell. I'll attach another testcase in a moment. BTW, that patch was even more rotten than either of us realized. Notice that the TableBackgroundRule no longer exists--that's what I was targetting when I modified the if statement in nsHTMLStyleSheet.cpp. I'm looking through Hyatt's changes (2001-05-31) now, and I recommend putting this fix on hold for a bit.

I was not able to regression test the patch because Viewer kept locking up when I tried to run rtest.sh baseline. I'm going to try wiping out my mozilla directory again and downloading & compiling over the weekend. I tested against the Extended testcase above and against David Baron's three table background tests in bug 4510--results were as expected.

Just a short quote from the CSS2 spec: 17.6.1 The separated borders model In this model, each cell has an individual border. The 'border-spacing' property specifies the distance between the borders of adjacent cells. *This space is filled with the background of the table element*. Rows, columns, row groups, and column groups cannot have borders (i.e., user agents must ignore the border properties for those elements). I think what IE is doing is just right, they inherit the color into the table cell. I think we should do the same. This would require from fantasai's patch only the nsTableFrame.cpp part in order to paint the cellspacing with the table backgrounds color. I have to admit that the CSS2 spec is not conflict free in this matter and as one can see by the David's posting http://lists.w3.org/Archives/Public/www-style/1999Jul/0083.html and the answers, the W3C is not able create a clear specification over two years now. So it is hard to believe that this will change very soon. The honest interpretation of the conflict is that for the collapsed border model as default the space between cell's should be covered by table- , row- , col- etc background, while for the separated border model it deviates from the default model and only the table background is used for the space between the cells. In term's of testing I prefer now to check backgrounds.html file in layout/tests/table/core against IE and the strict behaviour in order to get a first impression on the fix and the remaining problems, and only to run the regression tests when this looks fine.

Bernd, please copy your comments into bug 4510. This bug is quirks-specific-- we're not changing standard mode rendering here. > This would require from fantasai's patch only the nsTableFrame.cpp part in > order to paint the cellspacing with the table backgrounds color. Even if we decide to change standard mode to imitate quirks, the layout mode check still needs to go.

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

fantasai: I dont think that the last bug is really a dupe of this bug it is more a dupe of bug 41635. BTW IE renders this testcase fine and they do not need the paint stuff.

Changes only to nsTableFrame.cpp and quirk.css, as requested. And now, I request review. =)

I'll do the review + testing quote from the code reviewers guide: 'Remember, a code reviewer is not a friend. A good code reviewer can feel like your worst enemy.' :-)

here you go fantasai: r=bernd

sr=attinasi

fix checked in

Reopening because I think it caused the regression at bug 91034.

Closing as Fixed again. After all, it's not related to bug 91034.

Reopening to mark topembed.

patch was checked into the m0.9.2 branch.

Mass removing self from CC list.

Now I feel sumb because I have to add back. Sorry for the spam.