One little problem with your fix from last week. Although it isn't defined clearly in the spec, because of images (tiled or not tiled), when HTML's background is "transparent none" (this should include "transparent url()" when the image doesn't load), all background properties of HTML and BODY should be switched, so BODY becomes *transparent*. Otherwise image tiling/positioning gets messed up/duplicated, as it does above.

I was setting the BODY's background to transparent, but the problem was that PaintBackground() ignores the mBackgroundFlags member of the color struct and just checks for a non-empty background URL. So I changed the body code to set the background URL to an empty string. This should work in all cases

Verified fixed.

This has broken again. Reassigning to Buster. Here is David's formal description of what should be happening: # For HTML documents, however, we recommend that authors specify the # background for the BODY element rather than the HTML element. User # agents should observe the following precedence rules to fill in the # background: if the value of the 'background-color' property for the # HTML element is 'transparent' and the 'background-image' property for # the HTML element is 'none', then derive the actual value of each of # the following properties on the HTML element from the computed value # on the BODY element and derive the actual value on the BODY element # from the computed value on the HTML element: 'background-color', # 'background-image', 'background-repeat', 'background-attachment', and # 'background-position' (where the actual values must be recomputed # based on the size of the HTML element). If, once this is done, the # actual value of 'background-color' on the HTML element is # 'transparent', then the rendering is undefined. His reasoning is described here: http://lists.w3.org/Archives/Public/www-style/1999Sep/0026.html See also bug 13473, which will deal with what happens to the background of the canvas once this is fixed.

(Troy no longer with us.) A whole slew of relevant test cases can be found here: http://www.bath.ac.uk/%7Epy8ieh/internet/projects/mozilla/background/

Nom. nsbeta3, recc. nsbeta3+. Correctness of handling of background settings for HTML and BODY elements--whole-document bkgnds are a widely-used part of CSS1.

Taking this as it is closely related to 40984 which I am working on.

I have a fix for this one now. It is pretty straight forward: I'll attach the diff for review. The Box Acid Test passes, as do Ian's background tests (except for 4, 12, 16 which have minor issues with the images, and the fact that some of the tests with the rule '* { margin: 1em; padding: 1em; }' cause the scrollbar to get whacked bad - I think that is another bug, actually).

FYI, the scrollbar issue -- universal selector * matches XBL not in the DOM -- is already filed separately (can't recall the number right now).

Thanks, Ian. It is bug 21890, and pretty frightening too.

David, Ian, somebody else - can you review the patch for me? ekrock (and others) would really like to get this into PR2... Thanks.

Hmmm...looking at the patch very quickly, some comments: * [Nit] The first initialization of |styleColor| seems unused, why declare the variable outside of the block, and why initialize it twice before using it? * Why did you remove the 14 lines of code at the end of the patch? Is that done somewhere else? It seems necessary, because otherwise tiled backround images or background images with transparency will cause weird problems. I need to look at this a little more before saying r=, since I don't know this code. Maybe I'll try it out a little later...

Thanks for looking David. The initialization of styleColor is not necessary unless those 14 lines are put back in, so they should not be in the patch. Now, about those lost lines of code: Their purpose was to clear the values of the background properties on the BODY and HTML contexts after the values have been propogated up to the Canvas. However, when that is done the BODY can no longer have its own background and about half of Ian's tests fail. Also, if the background is propogated from the BODY to the canvas then it is not really sufficient to mark the body's background as PROPOGATED_TO_PARENT since it is not propogated to it's parent, it is propogated to its grandparent. After trying several different things it seemed to me that there was no need to change the style context of the body or html element after propogating their background up to the canvas. The point you bring up about the tiled images is valid, in fact the tests on Ian's site that have tiled images on the BODY end up looking wrong because the BODY and the canvas image tiles do no line up. However, if you do leave those 14 lines in then there are other problems, so maybe part of it needs to be done (like clearing only the background properties that were actually propogated to the canvas and leaving any that were not propogated alone - the flag-bits are not currently sufficient though). I'll look at it some more. Again, thanks.

Oops, small mistake: the styleColor has to be initialized initially for the code that follows the propogation of the background, starting with nsCOMPtr<nsIPresShell> presShell; The second initialization is only needed for the clearing of the background (which is not in the patch).

Don't you want the 14 lines to be in an " if (bodyStyleColor == styleColor) " (or many of the other possible equivalent statements)? Would that fix the problems? The code isn't perfect, but I think it would come close enough for beta2...

Not exactly... I think half should be in the then and half in the else...

I revised the patch as I mentioned in my previous comments. I ran through Ian's HTML-based tests, and I thought the results were reasonable (but not yet perfect). I need to figure out why I crash on the XML ones... (it's in code I just changed).

correctness of compliance with official W3C CSS1 test suite. Reviewers will closely evaluate success on this suite. Fixing this is key product goal. PDT please approve check-in.

David, I did a similar change last night and I agree that it is better. Instead of doing 'if (bodyStyleColor == styleColor) ... else ' I just use styleColor itself to clear the background: less code and it does the same thing. I'll attach the patch (cleaned up the comments too) and hopefully we can get this checked in. And you are right, the tiled images look much better now (since the Canvas' tiles show through the body instead of the body having its own). Thanks again for you help!

Nominating nsbeta2 since we have a good patch and ekrock has strong desire to get this in (see Additional Comments From ekrock@netscape.com 2000-07-20 10:40)

A few comments on the comments: * It's worth mentioning that the root->canvas propogation is really a separate issue (separate statement in the spec), and should probably be handled elsewhere to really get it right. * Are you sure the "XXX" comment that you removed might not be valid still? r=dbaron, assuming you've tested fixed backgrounds (e.g., http://www.webstandards.org/css/winie/ ) and they still work.

Reinstating changes I wiped away. Bugzilla collision detection doesn't work quite right anymore... (it says nothing changed but the comments!)

I have tested fixed backgrounds, however the page you mentioned (WinIE) does not have show a fixed background, and did not *before* the changes. I have other fixed-background tests, and just created one using the stylesheet from the WinIE page, and they work fine. I do not yet understand why the http://www.webstandards.org/css/winie/ page is not getting a fixed background image... but I'm looking into it. For an example of a fixed-background that works, try Ian's at http://www.bath.ac.uk/%7Epy8ieh/internet/projects/mozilla/fixedpos-bg-pos2.html

Never mind... that page is browser-sniffed because 4.x won't handle the stylesheet.

Good to know, because when I d'load the stylesheet locally (via IE5) the background-attachment is fixed and it works great. The stylesheet retrieved from Moz is indeed lacking the fixed attachment. Whew, thought I smelled a new bug... Thanks D. I guess this one is ready to commit now, waiting on PDT approval.

Putting on [nsbeta2-] radar. Not critical to beta2. Adding "nsbeta3" keyword for consideration of a fix for that milestone.

:( OK, I guess I will then include a further refinement wherein the propogation happens if the canvas background is non-transparent too, instead of just when the body or html has a non-transparent background. This fixes the problem of changing the background _to_ transparent from non-transparent via the DOM. Attaching testcase for DOM update, and then a (final?) patch.

milestone set

Clearing [nsbeta2-] to trigger re-evaluation. Folks, the urgency of accepting this fix has changed. WSP decided to cause a firedrill re: standards compliance and ship dates and there's a risk of negative press coverage on the topics of standards compliance, progress towards a finished product, etc. Given that, this is a very poor time for us to push NSB2 out the door with a known regression (even though we have a fix in hand!) in a widely-watched test of standards compliance such as the Box Acid Test--that runs the risk of providing easy grist for cheap shots by sensationalist reporters who want to do a slam on Netscape. Given that we have a fix in hand, please mark [nsbeta2+] and accept the patch to simplify any damage control I have to do. Please call me to discuss if you're considering minusing this again. Thanks

Marc: Is the HTML element made transparent in your patch? Because it shouldn't, if it is. Imagine a case where a child element is moved to a z-index position below the root element, overlapping it a bit. According to the spec, the root element's background should hide the DIV where it overlaps. Here is an example: SIDE VIEW: CANVAS | | :root | | | | | | | | <-- FRONT | | | | | | | | | | | | | | | | DIV | | z-index: -1 0 CORRECT RENDERING: bgbgbgbgbgbgbgbgbgbgbgbgbgbgbg bgbgbgbgbgbgbgbgbgbgbgbgbg` 'g bgb+--------------+gbgbgbg C g bgb| :root gbgbgbg|gbgbgbg A g bgb|bgbgbgbgbgbgbg|gbgbgbg N g bgb|bgbgbgbgbgbgbg|gbgbgbg V g bgb|bgbgbgbgbgbgbg|gbgbgbg A g bgb|bgbgbgbgbgbgbg|--+bgbg S g bgb|bgbgbgbgbgbgbg| |bgbg. ,g bgb|bgbgbgbgbgbgbg| |bgbgbgbg bgb+--------------+ |bgbgbgbg bgbgbg| |bgbgbgbg bgbgbg| |bgbgbgbg bgbgbg|DIV |bgbgbgbg bgbgbg+--------------+bgbgbgbg bgbgbgbgbgbgbgbgbgbgbgbgbgbgbg bgbgbgbgbgbgbgbgbgbgbgbgbgbgbg INCORRECT RENDERING: bgbgbgbgbgbgbgbgbgbgbgbgbgbgbg bgbgbgbgbgbgbgbgbgbgbgbgbg` 'g bgb+--------------+gbgbgbg C g bgb| :root gbgbgbg|gbgbgbg A g bgb|bgbgbgbgbgbgbg|gbgbgbg N g bgb|bgbgbgbgbgbgbg|gbgbgbg V g bgb|bgbgbgbgbgbgbg|gbgbgbg A g bgb|bg+-----------|--+bgbg S g bgb|bg| | |bgbg. ,g bgb|bg| | |bgbgbgbg bgb+--------------+ |bgbgbgbg bgbgbg| |bgbgbgbg bgbgbg| |bgbgbgbg bgbgbg|DIV |bgbgbgbg bgbgbg+--------------+bgbgbgbg bgbgbgbgbgbgbgbgbgbgbgbgbgbgbg bgbgbgbgbgbgbgbgbgbgbgbgbgbgbg I need to turn that into a real testcase...

Hmmm... I *think* the correct rendering is that the DIV is competely hidden by the background of the canvas. 9.9: The root element creates a root stacking context 14.1: The background of the box generated by the root element covers the entire canvas. So I think (although it's not clear, I admit) that the background of the root element, which covers the entire canvas, is drawn at the z-index of the root element (and the DIV in your example should be completely hidden).

Hmm. That doesn't sound good. I've always assumed the canvas to be at z-index: -oo ...but I can't find anywhere in the spec to back that up...

Putting on [nsbeta2+] radar for beta2 fix.

Ian, the background of the HTML element is made transparent in my patch. Actually, whichever of the BODY or HTML element is propogated up to the canvas is then given a transparent background. If we do not do this then there is a problem with tiled images lining up properly (since the origin of the canvas and the HTML element may be different). The patch takes care of the vast majority of the BODY and HTML backgrounds I have seen (many from your tests). If there are still issues with z-ordering and the root element then maybe we should create a new bug for it? I guess what I'm saying is I would like to get this checked in for beta2 as-is, and address the potential z-order issues afterward.

Fix checked in (nsHTMLBodyElement.cpp)

Since there is no QA contact, stealing bug.

Marc: The canvas background should be positioned using the HTML element's origin. So in the normal case, the effect should be the same whether HTML is transparent or not. One of the tests examines this, I'll file a new bug when I verify this with the next build.

Ian, is that test background test 15: (http://www.bath.ac.uk/%7Epy8ieh/internet/projects/mozilla/background/15.html)? Please verify but it seems to be correct.

In this morning's builds, pre your checkin, it is rendered wrongly. I have just changed the test case to be slightly clearer; could you have another look? Cheers.

Unfortunately, I think it is wrong. Astrophy starts in the top left corner of the viewport, not in the HTML box... I'll post a picture for ya to look at to make sure I am seeing it correctly.

Yeah, that's the same as before your fix. I'll file a new bug for that, yes?

Yes please, Ian, a new bug. Go ahead and assign it to me, por favor.

Filed bug 46446 for the new issue.

Verified with commerical bits from 2000-07-25. Nice one Marc! The only remaining issues on those tests are bug 46446 and bug 15405.