| Summary: | [CSS1] Background-attachment: scroll should scroll within the containing block | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gérard Talbot <browserbugs2> | ||||||||||
| Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | hyatt, ian, mitz | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 420+ | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | OS X 10.4 | ||||||||||||
| URL: | http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/BackgroundCenterOfBodyBugInIE7.html | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Gérard Talbot
2006-09-09 18:40:20 PDT
I don't believe this is a bug. When the HTML element has no background, the body's background propagates up to to the root element and from there to the underlying canvas. This behavior is intentional in HTML documents as authors have the expectation that the "body" of the document will cover the entire canvas. See section 14.2, starting at the third paragraph: "The background of the root element becomes the background of the canvas and covers the entire canvas, anchored at the same point as it would be if it was painted only for the root element itself. The root element does not paint this background again. For HTML documents, however, we recommend that authors specify the background for the BODY element rather than the HTML element. For HTML documents whose root HTML element has computed values of 'transparent' for 'background-color' and 'none' for 'background-image', user agents must instead use the computed value of those properties from that HTML element's first BODY element child when painting backgrounds for the canvas, and must not paint a background for that BODY element. This does not apply to XHTML documents." Leaving open while i double-check with the CSS WG though. Mr Hyatt, I believe this comes from: http://www.hixie.ch/tests/adhoc/css/background/09.html You may want to consult Ian "Hixie" Hickson too... just a friendly suggestion :) Unless I'm wrong (and I may be :) ), in CSS 2.1, the root element does not (and should not) stretch vertically to cover/fill up the viewport. So, resizing the viewport should not extend the root element. So that lime squared "+" image should remained vertically-centered in whichever containing block it has and is being used for background-positioning. Regards, Gérard I've added borders around <html> and <body> in the testcase (provided URL). The background is painted by the *canvas* and not by the root element. The canvas *does* cover the viewport. "The background of the root element becomes the background of the canvas and covers the entire canvas, anchored at the same point as it would be if it was painted only for the root element itself. The root element does not paint this background again." I suppose "anchored at the same point as it would be if it was painted only for the root element" implies maybe Safari is wrong. Hixie, your comment on this bug is desired. > I suppose "anchored at the same point as it would be if it was painted only for
> the root element" implies maybe Safari is wrong.
Yes, Safari is wrong. If you have 'background-position: bottom' on the root element then it should paint at the bottom of the root element, not the bottom of the viewport. That's what that sentence means. :-)
Created attachment 20541 [details]
test case
Created attachment 20542 [details]
patch
It's one of my first patches, so please be nice... I'm more than happy to learn and fix any mistakes I've made.
I ran it through the webkit tests and the hixie.ch background tests to check for regressions, and made sure to cross-reference with the w3c specs.
Comment on attachment 20542 [details]
patch
This is on the right track. Thanks for contributing. Comments below:
background-size and background-position are both supposed to be computed using the box specified by background-origin. This is CSS3 stuff so that may not really be obvious yet, but that's how it is supposed to work.
Your check for background-position is correct. However background-size is now taking rather funny values, since it could end up being the viewport's width/height - margins. background-size should be using the same width and height you're passing to background-position.
The destination rect for fixed backgrounds is also being potentially messed up by your modifications to make left/right non-zero. For example:
cw = pw + left + right;
Now that you put margins into left/right, the destination rect for fixed backgrounds will be too small.
Style nitpick: Put spaces between arithmetic expressions, e.g.,
height() + marginTop() + marginBottom()
and not
height()+marginTop()+marginBottom().
Created attachment 20543 [details]
patch (updated)
Ok, I've implemented the requested changes and added some variables (rw, rh) to make it better formatted.
Will update tomorrow with test cases for background size Created attachment 20557 [details]
patch (updated 2)
Ok, hopefully this is the final patch. I've modified my test case to also test background-origin for background-size on root element.
Also, while I was testing that I noticed that WebKit's implementation of background-size: 50% auto;, that is percentage width and auto height, was broken and so I just quickly fixed it... was there a reason why it was that way, which I missed? (The formula that was there for calculation did not make any sense what-so-ever.) There is no separate ticket for that and I didn't think it was worth creating one for a change of one variable.
No test case was needed for that one, as there is one in fast/backgrounds/size ... it's the number 18.
Comment on attachment 20557 [details]
patch (updated 2)
- h = bg->imageSize(style()->effectiveZoom()).height() * scaledWidth / bg->imageSize(style()->effectiveZoom()).width();
+ h = bg->imageSize(style()->effectiveZoom()).height() * w / bg->imageSize(style()->effectiveZoom()).width();
This change fixes computation of height, but isn't the corresponding width case also broken? You could adjust the background-size computation so that it computes values of w and h first (for fixed/percent). Then handle the auto cases for each after that. (That enables you to fix the width case also.)
So instead of
if (width is fixed)
else if (width is percent)
else if (width is auto)
if (height is fixed)
else if (height is percent)
else if (height is auto)
you could do:
if (width is fixed)
else if (width is percent)
if (height is fixed)
else if (height is percent)
if (width is auto && !height is auto)
else if (!width is auto && height is auto)
else if (width and height both auto)
Comment on attachment 20557 [details]
patch (updated 2)
Ah I see that the other case is not broken. I still think the cleanup is nice, but it's not necessary.
Fixed in r31912. Hello all, When using Safari 3.1.2 build 525.21 (XP Pro SP3), the lime + image vertically moves, changes its y-position. When I follow steps to reproduce, I do NOT get expected results. Can someone confirm my findings? When I try the provided testcase with the latest stable release Safari 4.0 build 530.17, I get expected results: as I resize vertically the browser window, the lime squared "+" image remains vertically in the middle of its containining block, which is the body. So, please ignore my previous comment. regards, Gérard |