Closed Bug 70831 Opened 21 years ago Closed 20 years ago

Table background color is incorrect


(Core :: Layout: Tables, defect)

Not set





(Reporter: Junk_HbJ, Assigned: dbaron)




(Keywords: regression, Whiteboard: 0.9 [fix in hand] have r=,sr=,a= ready to check in.)


(15 files)

525 bytes, text/html
576 bytes, text/html
617 bytes, text/html
5.58 KB, image/png
6.12 KB, image/png
1.98 KB, patch
Details | Diff | Splinter Review
527 bytes, text/html
629 bytes, text/html
884 bytes, text/html
915 bytes, text/html
2.81 KB, patch
Details | Diff | Splinter Review
1.80 KB, patch
Details | Diff | Splinter Review
890 bytes, patch
Details | Diff | Splinter Review
1.66 KB, patch
Details | Diff | Splinter Review
1.56 KB, patch
Details | Diff | Splinter Review
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; 0.9) Gecko/20010302
BuildID:    2001030205

The rows of the table should be orange (like the empty fields), but are rendered
white. Mozilla 0.8 renders correct, btw.

Reproducible: Always
Steps to Reproduce:
1. Type in the URL

That's it...

Actual Results:  White table rows

Expected Results:  Orange table rows
this is indeed a regression. The page renders fine with 2001-01-24-20. Win98 and
renders with white cells 2001-02-28. Adding a keyword. Confirming, a testcase
would be great.
Ever confirmed: true
Keywords: qawanted, regression
Actually, the background is not always displayed white, but as your default
background colour (I have a shade of lilac as default background colour for
exactly finding bugs like this).
Making a test case.
Keywords: makingtest
Looks like some problem with Quirks mode: Without a doctype or with the HTML 3.2
doctype, the bug occurs. With the HTML 4.01 strict doctype, the bug does not occur.

Is this an intended Quirk?
Keywords: makingtest
there have been three checkins in this time:
bug 68411
bug 44505
and dcone's checkin from 2702.
none of these checkins are responsible for this regression. It also horks viewer
demo #6 and two table regression tests inside  layout/html/tests/table/other.
2001-02-23-10 was still fine.
Adding others to the CC list who might have some insight.
The regression has been caused by the checkin for bug 67478. Backing out the
changes for this bug makes the rendering of the testcase fine, the viewer #6 and
the table testcase in the /tests/table/other directory. The automated regression
test does not catch this kind of error :-((.
I'm on the road and don't have a Mozilla test environment. I have no idea what's 
going on here, maybe Marc can figure it out. Is the BodyFixupRule being applied 
to the table cell for some reason?

Try adding a background color to the BODY element in the testcase and see if the 
result works in any build.
Wait, I know what's going on here. In quirks mode table cells inherit their 
background from the document background (emulating old NS4 bug). Due to bug 
67478, the document background used to be set to transparent as long as the HTML 
and BODY elements had no background color. The fix to 67478 makes the canvas get 
its background color set to the correct default background color. So, now we're 
inheriting the default background color into the cells.

The solution is to change the quirk code so that it doesn't inherit into table 
cells unless a background is explicitly specified on the HTML and/or BODY 
Reassigning to Pierre.
Assignee: karnaze → pierre
*** Bug 71048 has been marked as a duplicate of this bug. ***
*** Bug 72094 has been marked as a duplicate of this bug. ***
*** Bug 71775 has been marked as a duplicate of this bug. ***
Target Milestone: --- → mozilla0.9
Moving to m0.9
another url that shows this is
OS: Windows 98 → All
Hardware: PC → All
*** Bug 72424 has been marked as a duplicate of this bug. ***
And yet another URL showing this (from another duplicate):
QA contact update
QA Contact: chrisd → amar
Also seen while viewing
I think this bug should be marked INVALID because the document contains
several markup errors, one of them being absolutely fatal. With this one
fixed, the rows become orange again :

1. the document contains <html><body><html> ; this is the fatal error

2. one table start tag is invalid : <TABLE WIDTH="100%"CELLPADDING=2 ...
   a space is missing between the closing double-quote and CELLPADDING

3. the document contains a </>

Also seen on
Are you in right bug Daniel?

- does not have illegal <html> <body> <html>
- the table start tag is correct
- the testcase does not contain a <a>/

And as described also the 

resource:///res/samples/test6.html renders wrong.

Yes I am, click on the URL field in header of the bug.
Daniel, before you invalidate this bug just look on the testcase I mentioned,
and may be for reference with a NN4.7. I don't care either about broken external
pages, but this bug breaks the table layout regression testcase
layout/html/table/other/nested2.html and this exactly what concerns me. (Another
small hint: will love to reopen this bug if you really
close it as invalid, because that wrong inherited color is white under Winxx but
gray under Mac...). May be we should remove the URL.
and yes, i will reopen in a heartbeat. this happens on many pages and is plainly 
seen on win32 when you change your windows os background color to lime green.

there are 4 other urls w/in this one bug that illustrate this problem. don't be a 
This is a straightforward bug. We know what's causing it and what to do about 
it, someone just needs to write the code.
Sorry everybody, I neglected my bug list in the past few weeks.

This bug should obviously not be marked Invalid.  As Luke pointed out, it also 
happens on Yahoo.  I'll work on it when I'm done with the style system memory 
footprint reduction ("real soon now").
Severity: normal → major
*** Bug 75748 has been marked as a duplicate of this bug. ***
hwaara's patch in bug 46480  fixes this behaviour.
*** Bug 75887 has been marked as a duplicate of this bug. ***
Whiteboard: interested for 0.9
I have a fix.  It is related in some way to bug 69143.
Whiteboard: interested for 0.9 → 0.9 [fix in hand]
Attached patch fixSplinter Review
My fix doesn't work for cases like where there is 
background image and a background color, but I think it is safe and valuable 
enough to check it in for 0.9.
I sent a review request morning early this morning to but 
nobody replied.

Marc?  ChrisK?  ChrisW?  Brendan?  Any volunteer?  This is one of the bugs with 
the most dups.
Keywords: review
Wouldn't it be simpler to reset to transparent rather than to the closest
non-transparent background color?  I imagine the latter could do strange things
in the case of overflow (e.g., floating tables that extend below their parent).
Ok, sr= withdrawn. Marc, why don't you take over?
Resetting the background to transparent is what is done already in 
StyleColorImpl::ResetFrom(null) but since in quirks mode we cut off all 
inheritance for the second style resolution, we need to fetch the background from 
the nearest parent.  As a comment says, "this really blows".

Your comment made me think of another way of doing it, though.  We could save the 
computed style from the first style resolution and override the style from the 
second resolution only if the first one has a transparent background (meaning 
that no rules and html attributes applied to the table element).
dbaron's testcase can be simplified: the table doesn't need to be floating and 
all the colors should not be inherited.

I'm going to attach another testcase that shows that the table background should 
not come from the first non-transparent background but from the BODY indeed, and 
that the text color should not be inherited at all but set to the default.
Attached file better testcase
Why BODY?  Because NN 4.x completely messes up that test case?

I really think it ought to be transparent, which I think is what it was before,
right?  What exactly was it that caused it to stop being transparent?

(I also think this whole bit of code would be better off in quirks.css (or maybe
the prefs sheet) than in C++ -- I vaguely remember arguing about that around two
years ago...  and I think it's probably making us more quirky than we really
need to be.)
I'm going to attach another testcase with a background image that shows that:
- we never emulated this quirk correctly when an image is present.
- it is probably impossible to emulate all the quirks that occur with tables.

If we are stressed by time, I propose to checkin the attached fix which covers 
most of the usual cases.  And later we can work work on transparent backgrounds 
for tables over images even when they are embedded within a block with a non-
transparent background...
I think you're focusing on a bug that was in NN 4.x only (that tables tend to
end block containers -- more of a parser bug than a style bug) rather than the
accepted table quirk that is in IE 3+ and NN 4.x.
>Why BODY?  Because NN 4.x completely messes up that test case?

Sure.  The whole discussion is around Nav quirks, there is no bug in strict mode, 
and it appears that Nav makes the table background show the BODY background and 
nothing else.  Just beeing transparent doesn't cut it.  Inheriting the whole BODY 
color & background style doesn't either.

The solution cannot be in the table drawing code either because, as my latest 
testcase shows, the table ignores its parent's non-transparent background.  
Finally, I don't see how this quirk could be described using normal style 
declarations in quirks.css.  Run the testcase, you will see.

Again, I have a quick solution that covers the most obvious aspects of this bug.  
If want to ship the bits today or tomorrow, I recommend we 
take my fix rather than nothing.
> The solution cannot be in the table drawing code either because, as my latest 
> testcase shows, the table ignores its parent's non-transparent background.  

That's a bug in NN 4.x and is not present in IE.  Quirks mode is for quirks that
existing web pages depend on to be rendered correctly, and if the bug isn't in
IE then it's very unlikely that web pages depend on it.  (The bug you're looking
at is more of a parser-ish bug anyway.)

> Finally, I don't see how this quirk could be described using normal style 
> declarations in quirks.css.  Run the testcase, you will see.

Examine the quirk that is needed for existing web pages -- i.e., the one that's
common between NN 4.x and IE.  (See bug 1044 and related bugs for background.)
> I think you're focusing on a bug that was in NN 4.x only

No, I pointed out that aspect just to say that we will probably never emulate all 
the quirks, but the main point is that a TABLE within a DIV withing the BODY 
shows the BODY background, not the DIV background.

This can't be emulated in quirks.css and I don't think it can be done only within 
style resolution, or only within the table drawing code either.
I take your point that all these quirks are not in IE either.
If we play at denigrating each other's testcases, I can say that yours, showing a 
floating table in quirks mode, isn't very valid either because floating elements 
did not exist in Nav4.

Remains a better-than-nothing patch, if someone wants it.
I will attach another patch that's not going to make everybody happy but at least 
it seems to work with all the reported URLs, and with the different testcases 
before they got out of hands.
Attached patch better patchSplinter Review
Could someone please review?

My apologies for not asserting more vigorously the validity of the patch in my 
previous comment but we are talking about realtively weird quirks here.  The 
patch makes table inherit the background color from the nearest parent if it 
doesn't have an image.  The point that needs to be discussed is whether it should 
be applied to floats and fixed positioned elements.
I think the real problem here has something to do with the mDocumentColorRule
in nsHTMLStyleSheet.cpp (and its interaction with the changes for bug 67478). 
It's not clear why it was made to match elements in tables, since buster's
checkin comment (revision 3.189) refers to the BodyFixupRule.

As evidence, the following patch fixes the bug, although it probably breaks
other things (but I really don't know what).

Index: nsHTMLStyleSheet.cpp
RCS file: /cvsroot/mozilla/content/html/style/src/nsHTMLStyleSheet.cpp,v
retrieving revision 3.216
diff -u -d -r3.216 nsHTMLStyleSheet.cpp
--- nsHTMLStyleSheet.cpp	2001/02/19 12:55:27	3.216
+++ nsHTMLStyleSheet.cpp	2001/04/24 01:18:52
@@ -899,9 +899,6 @@
         nsCompatibility mode;
         if (eCompatibility_NavQuirks == mode) {
-          if (mDocumentColorRule) {
-            aResults->AppendElement(mDocumentColorRule);
-          }
I think the HTMLDocumentColorRule is a mostly vestigial (except perhaps for its
effect on tables) predecessor of the BodyFixupRule.  But what my fix does is
limits roc's patch to the BodyFixupRule for bug 67478 (which caused this
regression) to affecting only the canvas's style context and not the
HTMLDocumentColorRule as well.

I'm not sure whether I should propagate the foreground color anyway, though. 
Maybe it would be better to just use the bodyStyleColor in this part of the code
if the canvasStyleColor was neither the bodyStyleColor nor the htmlStyleColor
and leave everything else as is until there's a chance to investigate it
further.  (I think that would actually be directly undoing the original regression,
whereas my patch above is slightly different.)
I prefered to assume that the fix in bug 67478 was the right thing to do because 
the BodyFixupRule is a can of worms that many people stumbled over in the last 
several years, and it looked like we finally had reached a consensus on it, so I 
focused on fixing only what was broken: table backgrounds in quirks mode.

dbaron's patch should be reviewed by the people who participated to bug 67478: 
roc+moz, attinasi, kmcclusk.

My patch is limited to table code.  Anybody and their worms can review it.

Hey!  Two fixes for the same problem!  Isn't it a reviewer dream?
While I think the BodyFixupRule is pretty good right now, the code I changed was
the part of the BodyFixupRule code that sets up the HTMLDocumentColorRule, which
I think is at least partially vestigial code that predated BodyFixupRule and may
even be causing some other bugs that should probably be investigated.

The patch you proposed to the table style hack is not the right thing -- it just
adds code in one place to cover up a bug somewhere else, and it doesn't even do
that very well since it creates a whole cascade (no pun intended) of other bugs.
(I could easily come up with other testcases that your "better patch" still
breaks, and, belive it or not, authors will expect tables and floats to work
properly in quirks mode as NN 4.x loses market share.)  The table style hack
itself is something I think we need to reexamine (I think it could be simplified
greatly), and I'd rather not see yet another bug "fix" depend on it.
No problem: reassigned to you then.  Marc who wrote the BodyFixupRule and roc+moz 
who made the patch for bug 67478 should review the last proposed fix at
Assignee: pierre → dbaron
Blocks: 67478
On a second thought: r=pierre, hoping it can help to get this checked in for 
moz0.9.  I verified the testcases attached to the different bugs filed against 
the BodyFixupRule in the past, and the testcases attached to the present bug.  
Note that some of the testcases we both did here are not rendered similarly to 
Nav4 but rather like IE in the way that the foreground color from the body is 
inherited within the table.  But it's fine with me.  Thanks.
I like dbaron's fix but I think it should only apply in Quirks mode. In Strict 
mode we should set the DocumentForegroundColor and DocumentBackgroundColor 
OK -- I made the change that you suggested.

I think the real fix is to split up the 3 uses for HTMLDocumentStyleColor:
 1) We use it to implement document.fgColor and document.fgColor in HTML
documents when we have no body element (i.e, if the script is running before the
body element is created).  (This issue is messy -- I'm also wondering if the
code in the body element itself needs some fixing too, but that's another issue...)

 2) We use it to match the HTML element in the attribute stylesheet
(nsHTMLStyleSheet).  I think this predates the BodyFixupRule and can be removed
(and it may mess up user stylesheets in some cases).

 3) We use it to apply to inner table contents in quirks mode.  We should
reexamine whether this is really the best way to handle this, but if it is, then
perhaps the HTMLDocumentColorRule could be only for this and (1) could be
handled by something else (or vice versa, although I think the solution in (1)
probably doesn't get along with the BodyFixupRule).
I like that patch! Let's get it in! sr=roc+moz
I agree - see if the drivers will let it in (or I'll do that for you if you 
I just sent mail to drivers, letting them know that I'm giving a=roc+moz on 
their behalf :-).
r=attinasi for patch 32002
I hate to say this... but there was a minor typo in that patch.  (It probably
doesn't make much of a difference, but I'd prefer it as close to the way it was

Instead of (within the navquirks check):

+      documentStyleColor = htmlStyleColor;

I meant to say:

+      documentStyleColor = bodyStyleColor;

Could you re-review?
Uh, OK, I still like it. r=attinasi

This time I'll ask the important question: did you run through some BODY/HTML
background color tests?

Here are Ian's:

And the Box Acid Test would be good too.
Ian's tests look like they're the same as the current bugs Ian described should
be in them.  The boxacidtest is fine.
It's fine with me either way. sr=roc+moz
Whiteboard: 0.9 [fix in hand] → 0.9 [fix in hand] have r=,sr=,a= ready to check in.
I would be more comfortable if you could check against all the testcase as I did 
yesterday.  You can find them under the current bug (+ the URLs like, bug 67478, bug 17911, bug 44199, bug 2285, bug 
40217, bug 33710 and bug 2054.
I also checked most of the testcases on this bug and testcases on all of the
other bugs mentioned (in addition to Ian's test suite and the boxacidtest, which
I checked earlier) before checking in the fix at 2001-04-24 19:55 PDT.

I filed bug 77287 on myself to investigate the issues I mentioned with
Closed: 20 years ago
Resolution: --- → FIXED
 Verified with the test cases mentioned by Ian and the other test cases. Works 
fine for me.. Marking verified on Build ID : 2001060509

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