17911 - [Content Sink] Body bgcolor not working in XML

Status: VERIFIED FIXED

(Core :: XML, defect, P3)

Description

[Christine Hoffman]

Using build 1999110308 on Win 95. Also tested on Mac and Linux. Behavior is
cross platform

Steps to Reproduce:
1. Open attached file. Document uses HTML namespace. Body bgcolor is assigned a
value or red.

Expected result: Body should display in red.
Actual result: Body has not color.

This worked in previous 5.0 version (10/3, for example).

Comment 1

[Christine Hoffman]

Attachment: Test case to demonstrate problem

Comment 2

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

This is caused by having a BODY element that's not a child of the HTML element.
I suspect the body background fixup rule.  I think if the parent isn't HTML, you
should cancel the fixup rule...

Troy?

Comment 3

[Hixie (not reading bugmail)]

David's analysis is correct: when the document is changed so that the parent
element of the BODY element is an HTML element in the HTML namespace, the
background misteriously appears.

Comment 4

[troy]

Yes, the fixup rule should only apply for HTML documents (and hence the parent
is the document element)

Comment 5

[Nisheeth Ranjan]

Acceptign bug and setting target milestone to M16...

Comment 6

[Nisheeth Ranjan]

Moving bugs out by one milestone...

Comment 7

[Nisheeth Ranjan]

Move back into M16 because this is related to the XML/HTML content sink 
factoring work that I want to do in M16...

Comment 8

[Nisheeth Ranjan]

Moving XML/HTML content sink factoring related bugs out to M17...

Comment 9

[Nisheeth Ranjan]

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

Comment 10

[Nisheeth Ranjan]

Nominating for beta 3...

Comment 11

[ekrock's old account (dead)]

correctness. Is it a usable workaround to specify the background color on the
BODY in a (possibly external) CSS stylesheet instead?

Comment 12

[Peter Trudelle]

nsbeta3-, ->future, since a workaround exists.

Comment 13

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

Requesting reevaluation for nsbeta3
 1) I think this should be a reasonably simple fix to the BodyFixupRule (don't
do anything if HTML not parent)
 2) What workaround??

Comment 14

[Marc Attinasi]

Hmmmm. Works for me (background is red) using a trunk build from Wed 8/9 on NT
and Linux.

Can somebody confirm this please?

I agree with David, however, that the BodyFixupRule shoul donly apply if the
document is an HTML file. This can be done in the method
nsHTMLBodyElement::GetInlineStyleRules I believe, where the BodyFixupRule is
created. Nonetheless, I think this bug is WORKSFORME.

Comment 15

[Hixie (not reading bugmail)]

No, the BODY is *not* red. The root element is red.

The body is a one line element in that document.

Comment 16

[Marc Attinasi]

Oops - sorry, you are right, of course. I'll look at cutting off the
BodyFixupRule when the parent of the HTMLBodyElement is not the HTMLElement -
should be easy.

Comment 17

[Marc Attinasi]

Nisheeth, I hope you don't mind me taking this (it is Future'd) but I have a 
really simple fix for it... I'll attach it as a patch for consideration.

Comment 18

[Marc Attinasi]

Attachment: PATCH to fix this bug: avoid BodyFixupRule if parent of BODY is not HTML

Comment 19

[Marc Attinasi]

Simple patch attached. Can somebody please review?

Comment 20

[Marc Attinasi]

CC'ing David Baron - can you check out the attached patch?

Comment 21

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

Hmmm.  Don't you want to put the new stuff after the
|mInner.GetInlineStyleRules(aRules)| ?

Comment 22

[Marc Attinasi]

Good catch David, you are absoluttely right, thanks. 

Attaching a new patch that avoids the body fixup rule but still allows the 
inline style rules from the inner to be used.

Comment 23

[Marc Attinasi]

Attachment: Corrected patch

Comment 24

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

Hate to say this now... but one more thing.  If someone makes body the root in a
XML doc, won't this fail because the GetParent returns null?  So should you
initialize to FALSE and set to TRUE at the inside if the tag does match?

Comment 25

[Marc Attinasi]

David, if the body was the root element then we would end up creating the 
BodyFixupRule, which would be OK because it checks to make sure that the parent 
of the body is not null (the style contexts, not the content elements). However, 
it is better to check for it before creating the rule, as you suggested, and 
avoid the overhead of the rule since it won't apply anyway. One more time now...

Comment 26

[Marc Attinasi]

Attachment: YAP: avoid the creation of the BodyFixupRule if HTML is not the parent of BODY.

Comment 27

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

r=dbaron, as long as there are no regressions on Ian's background tests.  (Boy,
I'm picky...)

Comment 28

[Marc Attinasi]

Believe it or not, your pickiness is a tremendous asset! Thanks, David.

Comment 29

[Marc Attinasi]

Approving for beta3: fix in hand, reviewed and tested. Localized change that 
will improve correctness and eliminate a tiny amount of bloat on some XML 
documents.

Comment 30

[Marc Attinasi]

Fix checked in (nsHTMLBodyElement.cpp)

Comment 31

[Christine Hoffman]

Verified fixed with 8/23 build

Comment 32

[Hixie (not reading bugmail)]

see bug 73294 before reopening this bug