Closed Bug 46480 Opened 20 years ago Closed 19 years ago

(Quirks) Color not initializing to 'inherit' within tables

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.0

People

(Reporter: fantasai.bugs, Assigned: pierre)

References

Details

(Keywords: css1, testcase, Whiteboard: WONTFIX ? -- minor quirks mode unquirk)

Attachments

(13 files)

Overview:

	In Quirks mode, the 'color' property is not inherited into <tbody>, 
	<tr>, or <td> unless "color: inherit" is explicitly specified. 
	Apparently, this is done to simulate Navigator 4.x, which does not 
	inherit color through tables. Although the quirk that blocks color 
	inheritance from the container into the table may be necessary, I don't 
	think that inheritance should be blocked in any part of the chain 
	linking <table>, <tbody>, <tr>, and <td>. Applying a color style to any
	of these elements has no effect in Navigator, so I don't think any sites 
	will depend on it not working; there's no point in specifying a color in 
	the first place. However, MSIE 5 supports intra-table color inheritance
	(I haven't tested IE4), and I'm sure sites take advantage of that.

Steps to Reproduce:

	Open up testcase (to be attached shortly) in Mozilla.

Actual Results:

	Color only inherits if /every single element/ between the assigned color 
	property and the table cell has 'style="color: inherit"'. This includes 
	<tbody>, whether it physically appears or not.

Expected Results:

	Color should inherit properly through the table tags.

Tested on Mozilla nightly build (id: 2000071810) on Windows 2000

Additional Information:

	Bug detected by Matt P <mpetrick@verida.com> on July 26th build.
Attached file testcase
Attached file Test Case 2
css properties not being inherited:
color, background-repeat, .....im sure there are others
Copying Matt P's comments from n.p.m.style post:
 | does not work on:
 | Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; m17) Gecko/20000725

'background-repeat' does not inherit. 

CSS1:5.3.4 - http://www.w3.org/TR/REC-CSS1#background-repeat
CSS2:14.2.1 - http://www.w3.org/TR/REC-CSS2/colors.html#background-properties
Keywords: css1, testcase
Would it be better to just make the backgrounds in quirks mode work the same
waythey do in strict mode?  That is, just paint the background on all table
elements...

There's another bug on this somewhere in the 4000's or 5000's.

Also, this is CSS2, not CSS1, since CSS1 did not describe tables.
Keywords: css1css2
Ignore my previous comment.  I saw the background-repeat stuff and thought the
bug was about background-color, not color.  Sorry.
Keywords: css2css1
QA Contact: desale → chrisd
Status: NEW → ASSIGNED
This form here needs an undo button...
Many apologies; I didn't mean to do that.
Reassigning to pierre.
Assignee: karnaze → pierre
Status: ASSIGNED → NEW
Target Milestone: --- → mozilla1.0
This bug is caused by the line marked below, which should probably only be 
invoked on <table> and not any of the other tags.

(layout/html/style/src/nsHTMLStyleSheet.cpp)

      // add the quirks background compatibility rule if in quirks mode
      // and/or the Table TH rule for handling <TH> differently than <TD>
      else if ((tag == nsHTMLAtoms::td) ||
               (tag == nsHTMLAtoms::th) ||
               (tag == nsHTMLAtoms::tr) ||
               (tag == nsHTMLAtoms::thead) || // Nav4.X doesn't support row
                                              // groups, but it is a lot
               (tag == nsHTMLAtoms::tbody) || // easier passing from the table
                                              // to rows this way
               (tag == nsHTMLAtoms::tfoot)) {
        // add the rule to handle text-align for a <th>
        if (tag == nsHTMLAtoms::th) {
          aResults->AppendElement(mTableTHRule);
        }
        nsCompatibility mode;
        aPresContext->GetCompatibilityMode(&mode);
        if (eCompatibility_NavQuirks == mode) {
          if (mDocumentColorRule) {
!>>         aResults->AppendElement(mDocumentColorRule);
          }
          aResults->AppendElement(mTableBackgroundRule);
        }
      }

I believe it was added to fix bug 1044.
OS: Windows NT → All
Hardware: PC → All
QA contact update
QA Contact: chrisd → amar
I investigated some and it seems like this isn't belonging to Quirks:

http://www.w3.org/TR/html4/struct/tables.html#h-11.2.6 - tells you that tables
doesn't have any implicit color= attribute, as someone suggested.

http://www.w3.org/TR/REC-CSS1#color - supports that all color attributes, should
be color: inherit;

Given this, I will attach a patch that fixes this bug, and adds the necessary
code to html.css

Need r= and sr=...
Keywords: patch, review
Attached patch fix v1Splinter Review
Color should not be set to 'inherit' in the html stylesheet. It is and should be 
done internally by the style system. The inheritance within tables is merely 
overridden by nsHTMLStyleSheet (which also overrides the rules in your patch).

> I investigated some and it seems like this isn't belonging to Quirks:

Mozilla inherits color properly in Standard mode, so...
What do you mean by "this"?
This bug seems to be related to bug 70831
Here comes a fix for it in nsHTMLStyleSheet.cpp

According to Bernd Mielke this patch also fixes bug 70831. But I feel that the
code change is quite risky, so I would appreciate if as many as possible looks
at the code and/or tries it on their build.

fantasai, does this do what you want?
Attached patch fix v2Splinter Review
> fantasai, does this do what you want?

No, the mTableBackgroundRule is for something else.

I have a patch that fixes this bug; it just doesn't pass the regression tests. 
(I haven't gotten around to sorting through the results.) If you want, I'll send 
it to you. There's no reason for you to waste time guessing how I expect to fix 
this. :)

BTW, I'm getting the impression that you are not testing your fixes. 
Are you able to compile Mozilla?
Yes, you can send it to me and yes (of course!) I compile and run my fixes.
> and yes (of course!) I compile and run my fixes.

And test them, too, I hope.
That CSS fix had no effect on my copy of Mozilla, so I'd like
to know what changed in the rendering of the testcase to make 
it seem that the bug had been fixed on your computer.
Whiteboard: WONTFIX ? -- minor quirks mode unquirk
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=28736 creates a problem
when
http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug6933.html
is viewed with the viewer and the standard DTD is selected. -> no go for this
patch :-(
How could attachment 28736 [details] [diff] [review], which moves a line out of an "if (quirks mode)", fix
bug 70831, which is quirks-mode only?
Attached patch patchSplinter Review
> WONTFIX ? -- minor quirks mode unquirk

This is not unquirking a quirk. It's fixing a bug in the implementation of a 
quirk.
*** Bug 71529 has been marked as a duplicate of this bug. ***
Attached image wfm WIN98
There is a regression in the behavior of the table color quirk (I haven't filed 
it yet), but this bug still exists. Please use the testcase I have just 
attached.

The patch needs to be updated to account for Hyatt's changes. I'll try to do 
that in the next few days.
Keywords: patch, review
Attached patch new patchSplinter Review
Requesting reveiw.
Keywords: patch, review
So, the patch just limits the application of the DocumentColorRule to the table
element, right? (wanna make sure I'm not missing something subtle here)
Precisely. Unless I've somehow mangled the if statements to do otherwise.. :)
Bernd asked me to explain exactly what this patch does. So I'm first going to 
explain what the affected code does, and then what I've changed.

mDocumentColorRule holds the color of the <body> element. There is a table 
NavQuirk that causes tables to use the color of the <body> element, so to 
imitate that in Mozilla, the mDocumentColorRule is applied to the table in 
quirks mode.

Specifically, the mDocumentColorRule was applied to <tbody>, <thead>, <tfoot>, 
<tr>, <td>, and <th> instead of just to the <table>. This was probably because 
it was a convenient place to put it since another rule for handling table 
backgrounds was applied to all these elements.

Setting a table color on <table> did not inherit into the table cell because the 
color got reset in the table row group and again in the table row and again in 
the table cell. Notice that in the testcase, the color does inherit properly if 
one explicitly specifies inherit for all these table elements.

The first patch I attached moved the mDocumentColor rule out of the table 
elements if check into a separate <table> check. When Hyatt checked in his style 
 performance fix on 2001-05-31, along with other changes he removed the table 
background rule from nsHTMLStyleSheet.cpp and put it in quirk.css. Since the 
table elements if check is no longer needed for the table background rule or the 
Quirk color rule, I have removed it. The <th> rule is moved out to its own 'else 
if', and the Quirk <table> color rule is likewise moved out.

The only other thing I changed was the order of if statements for the Quirk 
rule. It checks for the mDocumentColorRule before making calls to find the 
layout mode. That is all.

Any questions?
[s]r=attinasi - thanks!
I dont know how 'nörgeln'  translates into english, but I know how to do it.
In the first table the border is black instead of green. I think it should be
green as the div specifies color:green. 
from css2:
If an element's border color is not specified with a border property, user
agents must use the value of the element's 'color' property as the computed
value for the border color.
 
The table's border is black because the table's 'color' is black. As I've
explained above, Quirks mode breaks color inheritance on the table, using the
<body>'s color. Since the <body>'s 'color' is black, the table's color is black.

If I specify "table {color: inherit}", the border will be green, and the color
will inherit through the table as described by CSS (if you have the patch
applied).
r = bernd
fix checked in
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.