47127 - BODY bg fixup rule causes inheritance to fail (background-color)

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Hixie (not reading bugmail)]

Imagine that the BODY is given a background-image and a background-color.
If a child element then inherits the background-color, it should override the
transparent background of the child and thus hide the background-image.

Unfortunately, at the moment the BODY fixup rule is being applied before the
inheritance is taken care of, and thus the value of background-image that is
inherited is the 'transparent' value that BODY is given.

Since it should always be the computed value that is inherited, this is 
technically not incorrect. (cc'ing David so he can disagree.) But I don't think 
authors will like that line of reasoning one little bit!

TO REPRODUCE:
   Use testcases provided or the third paragraph of
   http://www.w3.org/Style/CSS/Test/current/sec71.htm

TESTED ON:
   Windows 2000 commericial bits 6.0.17.2000072920.

Nominating for nsbeta3. This is a standards compliance issue that shows up on
a very high profile test suite.

Comment 1

[Hixie (not reading bugmail)]

Attachment: Pawprint background image for testcase

Comment 2

[Hixie (not reading bugmail)]

Attachment: Testcase showing problem in detail along with 7 other cases that work.

Comment 3

[Marc Attinasi]

Will it never end?! It looks like the style context has to be smarter about how 
it inherits the backround color. Specifically, it needs to check if the parent 
context's background flags have been set to PROPOGATE_TO_PARENT and follow that 
propagation trail up and get the inherited background value from there. 

I am wondering if the background 'propagation to the canvas' approach is really 
the best way to go, since it continually suffers from minor breakdowns...

Comment 4

[Marc Attinasi]

Another possibility is to have the rendering code walk the propagation-chain up 
when it sees a transparent background. This would probably fix the test, but it 
would not be very efficient, especially if the inheritance is many levels deep. 
Also, this will be incorrect from the perspective of the CSS-OM: when the 
background value of an element's style is requested, it should get the computed 
value, so if the BODY's child has 'inherit' and we only fix it up on rendering 
then we are not doing any favors for the OM (it would have to walk toe 
propagation chain like the rendering code does to get the computed value). To 
really get the OM part correct we need to take to opposite approach: leave the 
style context for the BODY alone, and fixup the background (propagate it to the 
canvas) in rendering.

Comment 5

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

The second solution you propose is probably worse than this bug - it would cause 
problems with tiled backgrounds and elements with overflow.  It's also wrong...

Comment 6

[ekrock's old account (dead)]

[nsbeta3+].

Comment 7

[Marc Attinasi]

It looks like the problem is actually pretty simple: MapDeclarationColorInto 
takes into account the NS_STYLE_BG_PROPAGATED_TO_PARENT flag and walks up the 
context tree to get the color from the parent, however in the case of the BODY 
element this is not enough - we need to walk up to the grandparent (the canvas) 
to get the color. The fix is to change the code in MapDeclarationColorInto to 
walk up the contexts until the NS_STYLE_BG_PROPAGATED_TO_PARENT bit is no longer 
set, and to set that bit on the HTML's style context in the BodyFixupRule when 
the BODY's background is pushed to the canvas. Testing it now...

Comment 8

[Marc Attinasi]

Attachment: Patch to fix it: better handling of propagation bits (bonus MLK fix too).

Comment 9

[Marc Attinasi]

Attachment: Additional required patch: propagation bit has to be taken into account in PaintBackground too...

Comment 10

[Marc Attinasi]

Fix checked in: nsCSSRendering.cpp, nsCSSStyleRule.cpp, nsHTMLBodyElement.cpp

Comment 11

[Hixie (not reading bugmail)]

VERIFIED FIXED with Windows 2000 and Linux Commercial builds 20000090808.