Backgrounds on BODY and HTML

VERIFIED FIXED in M18

Status

()

P2
normal
VERIFIED FIXED
20 years ago
19 years ago

People

(Reporter: dbaron, Assigned: attinasi)

Tracking

({css1, regression})

Trunk
Other
Other
css1, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta2+] [FIX IN HAND], URL)

Attachments

(7 attachments)

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.

Updated

20 years ago
Status: NEW → ASSIGNED

Updated

20 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 20 years ago
Resolution: --- → FIXED

Comment 1

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

Updated

20 years ago
Status: RESOLVED → VERIFIED
(Reporter)

Comment 2

20 years ago
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.
Blocks: 13473
Status: VERIFIED → REOPENED
Resolution: 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/
Assignee: troy → buster
Status: REOPENED → NEW
(Reporter)

Updated

19 years ago
Keywords: css1
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.
Keywords: nsbeta3

Updated

19 years ago
Depends on: 40984

Updated

19 years ago
No longer depends on: 40984
Keywords: regression

Updated

19 years ago
Depends on: 40984
(Assignee)

Comment 6

19 years ago
Taking this as it is closely related to 40984 which I am working on.
Assignee: buster → attinasi
(Assignee)

Comment 7

19 years ago
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).
Status: NEW → ASSIGNED
(Assignee)

Comment 8

19 years ago
Created attachment 11540 [details] [diff] [review]
PATCH to get the HTML and BODY background correctly onto the canvas
FYI, the scrollbar issue -- universal selector * matches XBL not in the DOM -- 
is already filed separately (can't recall the number right now).
(Assignee)

Comment 10

19 years ago
Thanks, Ian. It is bug 21890, and pretty frightening too.
(Assignee)

Comment 11

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

Comment 13

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

(Assignee)

Comment 14

19 years ago
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).
Created attachment 11611 [details] [diff] [review]
the above patch with the "14 lines" added back in, modified
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.
Keywords: correctness
(Assignee)

Comment 20

19 years ago
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!
(Assignee)

Comment 21

19 years ago
Created attachment 11640 [details] [diff] [review]
YAP: mostly the same as previous, cleaned up slightly (comments updated too).
(Assignee)

Comment 22

19 years ago
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)
Keywords: nsbeta2
Whiteboard: [FIX IN HAND]
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.
Keywords: nsbeta2
Whiteboard: [FIX IN HAND]
Reinstating changes I wiped away.  Bugzilla collision detection doesn't work
quite right anymore... (it says nothing changed but the comments!)
Keywords: nsbeta2
Whiteboard: [FIX IN HAND]
(Assignee)

Comment 25

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

Comment 27

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

Comment 28

19 years ago
Putting on [nsbeta2-] radar. Not critical to beta2.  Adding "nsbeta3" keyword 
for consideration of a fix for that milestone. 
Whiteboard: [FIX IN HAND] → [nsbeta2-] [FIX IN HAND]
(Assignee)

Comment 29

19 years ago
:(

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.
(Assignee)

Comment 30

19 years ago
Created attachment 11648 [details]
Testcase allowing DOM updates to background color and image
(Assignee)

Comment 31

19 years ago
Created attachment 11649 [details]
image file for testcase
(Assignee)

Comment 32

19 years ago
Created attachment 11651 [details] [diff] [review]
YAP: takes care of DOM updates to clear the background too
(Assignee)

Comment 33

19 years ago
milestone set
Target Milestone: --- → M18
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
Whiteboard: [nsbeta2-] [FIX IN HAND] → [FIX IN HAND]
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...
Whiteboard: [FIX IN HAND] → [FIX IN HAND] (py8ieh:need evil test)
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...

Comment 38

19 years ago
Putting on [nsbeta2+] radar for beta2 fix.
Whiteboard: [FIX IN HAND] (py8ieh:need evil test) → [nsbeta2+] [FIX IN HAND] (py8ieh:need evil test)
(Assignee)

Comment 39

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

Comment 40

19 years ago
Fix checked in (nsHTMLBodyElement.cpp)
Status: ASSIGNED → RESOLVED
Last Resolved: 20 years ago19 years ago
Resolution: --- → FIXED
Since there is no QA contact, stealing bug.
QA Contact: py8ieh=bugzilla
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.
(Assignee)

Comment 43

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

Comment 45

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

Comment 46

19 years ago
Created attachment 11854 [details]
Picture of background test 15 for Ian
Yeah, that's the same as before your fix. I'll file a new bug for that, yes?
(Assignee)

Comment 48

19 years ago
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.
Status: RESOLVED → VERIFIED
Whiteboard: [nsbeta2+] [FIX IN HAND] (py8ieh:need evil test) → [nsbeta2+] [FIX IN HAND]
You need to log in before you can comment on or make changes to this bug.