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

RESOLVED FIXED in mozilla1.0

Status

()

P3
normal
RESOLVED FIXED
19 years ago
18 years ago

People

(Reporter: fantasai.bugs, Assigned: pierre)

Tracking

({css1, testcase})

Trunk
mozilla1.0
css1, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: WONTFIX ? -- minor quirks mode unquirk)

Attachments

(13 attachments)

(Reporter)

Description

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

Comment 1

19 years ago
Created attachment 11933 [details]
testcase

Comment 2

19 years ago
Created attachment 11957 [details]
Test Case 2

Comment 3

19 years ago
css properties not being inherited:
color, background-repeat, .....im sure there are others
(Reporter)

Comment 4

19 years ago
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: css1 → css2
Ignore my previous comment.  I saw the background-repeat stuff and thought the
bug was about background-color, not color.  Sorry.
Keywords: css2 → css1

Updated

18 years ago
QA Contact: desale → chrisd
(Reporter)

Updated

18 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 7

18 years ago
This form here needs an undo button...
Many apologies; I didn't mean to do that.

Comment 8

18 years ago
Reassigning to pierre.
Assignee: karnaze → pierre
Status: ASSIGNED → NEW
Target Milestone: --- → mozilla1.0
(Reporter)

Comment 9

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

Comment 10

18 years ago
QA contact update
QA Contact: chrisd → amar

Comment 11

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

Comment 13

18 years ago
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"?

Comment 14

18 years ago
This bug seems to be related to bug 70831

Comment 15

18 years ago
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?
(Reporter)

Comment 17

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

Comment 18

18 years ago
Yes, you can send it to me and yes (of course!) I compile and run my fixes.
(Reporter)

Comment 19

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

Comment 20

18 years ago
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?
(Reporter)

Comment 22

18 years ago
Created attachment 33303 [details] [diff] [review]
patch
(Reporter)

Comment 23

18 years ago
Created attachment 33304 [details]
table regression test results [edited]
(Reporter)

Comment 24

18 years ago
Created attachment 33305 [details]
quirk test [text inside both tables should be blue]
(Reporter)

Comment 25

18 years ago
> WONTFIX ? -- minor quirks mode unquirk

This is not unquirking a quirk. It's fixing a bug in the implementation of a 
quirk.
(Reporter)

Comment 26

18 years ago
*** Bug 71529 has been marked as a duplicate of this bug. ***

Comment 27

18 years ago
Created attachment 39803 [details]
wfm WIN98
(Reporter)

Comment 28

18 years ago
Created attachment 39816 [details]
Testcase with workaround for quirk regression
(Reporter)

Comment 29

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

Comment 30

18 years ago
Created attachment 40564 [details] [diff] [review]
new patch
(Reporter)

Comment 31

18 years ago
Requesting reveiw.
Keywords: patch, review

Comment 32

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

Comment 33

18 years ago
Precisely. Unless I've somehow mangled the if statements to do otherwise.. :)
(Reporter)

Comment 34

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

Comment 35

18 years ago
[s]r=attinasi - thanks!

Comment 36

18 years ago
Created attachment 41271 [details]
ie rendering of the testcase

Comment 37

18 years ago
Created attachment 41272 [details]
mozilla rendering of the inheritance testcase

Comment 38

18 years ago
Created attachment 41273 [details]
ooops now comes the ie rendering

Comment 39

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

Comment 40

18 years ago
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).

Comment 41

18 years ago
r = bernd

Comment 42

18 years ago
fix checked in
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.