Closed
Bug 509681
Opened 15 years ago
Closed 15 years ago
Gradients as body background should fill the browser window
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
mozilla1.9.3a1
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: manfred, Assigned: ventnor.bugzilla)
References
()
Details
(Keywords: verified1.9.2)
Attachments
(4 files, 2 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
dbaron
:
approval1.9.2+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10_5_8; de-de) AppleWebKit/530.19.2 (KHTML, like Gecko) Version/4.0.2 Safari/530.19
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.2a1) Gecko/20090806 Namoroka/3.6a1
When a gradient is set as background for the body, then it should be at least as large as the browser window.
Reproducible: Always
Steps to Reproduce:
1. Make a gradient the background of the body.
2. Make the browser window larger than the body text.
Actual Results:
Ugly white area appears at the bottom of the browser window.
Expected Results:
The gradient should fill the whole browser window.
Background colors and background images fill the whole browser window. Therefore background gradients should do the same. They do in Safari.
Comment 1•15 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a2pre) Gecko/20090810 Minefield/3.6a2pre
Looks indeed weird.
Component: General → Style System (CSS)
OS: Mac OS X → All
Product: Firefox → Core
QA Contact: general → style-system
Hardware: PowerPC → All
Version: unspecified → Trunk
Updated•15 years ago
|
Flags: blocking1.9.2?
Updated•15 years ago
|
Assignee | ||
Comment 2•15 years ago
|
||
Without no-repeat, the gradients repeat for the length of the window. bgOriginRect is being reported wrong, or is this expected? I suggest using the root element rather than the body.
Assignee | ||
Comment 3•15 years ago
|
||
I think this is purely because of a difference between what Gecko and Webkit consider the "body". If you set the gradient rule on the root element, I think it should work out fine.
Comment 4•15 years ago
|
||
For HTML pages, "body" has always been magic. When the html element doesn't have a background, the body's background must propagate to the html element.
Comment 5•15 years ago
|
||
Backgrounds on body (when none is on the root) vs. on the root should behave indistinguishably. If moving it to the root makes a difference, that's a bug. See the third and fourth paragraphs of http://www.w3.org/TR/CSS21/colors.html#background .
Assignee | ||
Comment 6•15 years ago
|
||
OK, never mind then. I think I found the problem anyway.
Assignee | ||
Comment 7•15 years ago
|
||
Some code was not being run when it should.
Assignee: nobody → ventnor.bugzilla
Attachment #393949 -
Flags: review?(dbaron)
Could this just be bug 507939?
Assignee: ventnor.bugzilla → nobody
I suppose if Michael sees it, then no.
Comment 10•15 years ago
|
||
You should define the behavior of gradients in terms of CSS3's background positioning and painting areas. Read the comment at the top of the method and determine precisely where you want any deviation to fit; just moving code like this is going to break other things. Specifically, the patch you propose here, which I assume you have not run against layout/reftests/backgrounds reftests, will incorrectly place percent-positioned scaled background images if I read it right -- there is at least one reftest which checks this.
Assignee | ||
Comment 11•15 years ago
|
||
Comment on attachment 393949 [details] [diff] [review]
Patch
I suppose roc could do this if dbaron doesn't get to it first.
Attachment #393949 -
Flags: review?(roc)
Assignee | ||
Comment 12•15 years ago
|
||
Comment on attachment 393949 [details] [diff] [review]
Patch
Didn't see Jeff's comment.
Attachment #393949 -
Flags: review?(roc)
Attachment #393949 -
Flags: review?(dbaron)
Assignee | ||
Comment 13•15 years ago
|
||
(In reply to comment #10)
> You should define the behavior of gradients in terms of CSS3's background
> positioning and painting areas. Read the comment at the top of the method and
> determine precisely where you want any deviation to fit; just moving code like
> this is going to break other things. Specifically, the patch you propose here,
> which I assume you have not run against layout/reftests/backgrounds reftests,
> will incorrectly place percent-positioned scaled background images if I read it
> right -- there is at least one reftest which checks this.
The code that I move is completely independent from all the code above it. All it does is make sure gradients get the right bgPositioningRect, if it does break things like you say then I wouldn't know why. I'll re-run the reftests to double-check, but I don't see how this code move could break since the only interaction is gradients reading the new bgPositioningRect.
Comment 14•15 years ago
|
||
Oh, hm, I misread the patch, that's a different hunk from what I thought it was. You may actually be safe; I'd have to look closer than I actually did.
Assignee | ||
Comment 15•15 years ago
|
||
OK, this presents an interesting problem. If you set an image URL then do -moz-background-size 100% 100%, you get the exact same problem here with images. I'm not really sure what to do.
Assignee | ||
Updated•15 years ago
|
Attachment #393949 -
Attachment is obsolete: true
Comment 16•15 years ago
|
||
Not true re image URLs and background-size: 100% 100%. Percent background-sizes are relative to the dimensions of the background positioning area, which is the element itself. If you want the percentages to be determined relative to the entire window, you have to have fixed background-attachment to change the background positioning area to be the entire window.
Assignee | ||
Comment 17•15 years ago
|
||
Oh, so our current behaviour is correct if the bg is not fixed?
For backgrounds that have been propagated to the viewport, the background positioning area should be the whole window. It's as if the background was really on the viewport, not on the root or body elements.
Assignee | ||
Comment 19•15 years ago
|
||
This works right.
Assignee: nobody → ventnor.bugzilla
Attachment #394003 -
Flags: review?(roc)
It seems to me that if the root element has a margin then this patch is going to draw the gradient background with the size of the canvas frame, but positioned at an offset to the canvas frame, which seems wrong.
It also seems to me that our current code is correct insofar as the gradient's intrinsic size should be the background positioning area.
I don't know what the correct behaviour is here. I think this is a spec issue. The spec needs to clarify what the background positioning area is for backgrounds that have been propagated to the viewport. I think a reasonable option would be to do the same as for background-attachment:fixed and make the background positioning area the initial containing block. A slightly better option might be to make the background positioning area be the entire area that can be scrolled into view by the viewport scrolling mechanism (or the initial containing block if there is no such mechanism).
Comment 21•15 years ago
|
||
What part of
http://dev.w3.org/csswg/css3-background/#special-backgrounds
# The background of the root element becomes the background of the canvas
# and its background painting area extends to cover the entire canvas,
# although any images are sized and positioned relative to the root element
# as if they were painted for that element alone. (In other words, the
# background positioning area is determined as for the root element.)
is unclear?
I can't spec the behavior you want because of
http://www.w3.org/TR/CSS21/colors.html#background
# The background of the root element becomes the background of the canvas
# and covers the entire canvas, anchored (for 'background-position') 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 just missed that, sorry.
That makes this bug INVALID, doesn't it?
Assignee | ||
Comment 23•15 years ago
|
||
Webkit does fill the entire viewport when a gradient is set on the body. I'm pretty sure it will stretch to the entire page if part of the page is offscreen. I'm not sure what it does if a margin is specified on the root element.
That may be so but they don't necessarily get this right.
Try a test which gives the root element border, padding and margins, and try using a background image with background-position:0 0 and background-position:100% 100%. That should be positioned within the root element borders, according to the spec fantasai quoted in comment #21. If Webkit gets that right, then they must give gradient backgrounds special treatment that doesn't use the background positioning area to size or position the gradient.
Comment 25•15 years ago
|
||
(In reply to comment #24)
WebKit propagates the gradient background to the viewport (the same as background-color). But I don't think Gecko gets the gradient correctly.
two tests, background on <body>:
with gradient
http://dev.l-c-n.com/_temp/background-gradient-root2.html
with background-image
http://dev.l-c-n.com/_temp/background-gradient-root.html
Comment 26•15 years ago
|
||
root element with border/padding/margin
body with gradient background
Comment 27•15 years ago
|
||
On the left, Minefield 20090812 nightly, on the right WebKit latest nightly.
Your gradient testcases are using background-attachment:fixed, which forces the background positioning area to be the initial containing block, which is not the case we're dealing with in this bug.
Sorry, actually, the original bug report here *is* for background-attachment:fixed. Webkit behaves the same way we do for background-attachment:normal. So all this discussion was pointless and we clearly do have a bug...
Attachment #394003 -
Flags: review?(roc) → review-
Attachment #393949 -
Flags: review+
Comment on attachment 393949 [details] [diff] [review]
Patch
This patch was exactly right. Since the intrinsic size of the gradient depends on the background positioning area, we need to finish computing the background positioning area before we set the gradient intrinsic size.
We do need tests here though.
Attachment #393949 -
Attachment is obsolete: false
Assignee | ||
Comment 31•15 years ago
|
||
Attachment #393949 -
Attachment is obsolete: true
Attachment #394003 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 32•15 years ago
|
||
Updated to tip.
Attachment #394213 -
Attachment is obsolete: true
Assignee | ||
Comment 33•15 years ago
|
||
Dao, would you be able to check this in at any time?
This'll probably also need approval1.9.2 if the blocking flag isn't set soon...
Comment 34•15 years ago
|
||
I can land it. Am I correct that the commit message should say that it's fixing the area used for background-position:fixed gradients? (I don't think it has anything to do with body anymore.)
Yes.
It's actually fixing the size of the gradient image, specifically.
Comment 37•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/909161bf21e5
Not sure if that makes this bug fixed, though.
Comment 38•15 years ago
|
||
Comment on attachment 394213 [details] [diff] [review]
Now with tests
And a1.9.2=dbaron (I suspect you'll have to use this version for the 1.9.2 landing)
Attachment #394213 -
Flags: approval1.9.2+
Assignee | ||
Comment 39•15 years ago
|
||
Comment on attachment 394213 [details] [diff] [review]
Now with tests
Indeed I do. Would you or someone else be able to check it in?
Attachment #394213 -
Attachment is obsolete: false
Assignee | ||
Updated•15 years ago
|
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs 1.9.2 checkin]
Assignee | ||
Updated•15 years ago
|
Attachment #395999 -
Attachment description: Rebased → Rebased (checked into m-c)
Comment 40•15 years ago
|
||
Just an FYI for crossbrowser testing, since I ran into it...either webkit or opera (the latter IIRC) incorrectly defaults 2nd background-size parm to be same as 1st parm, instead of auto.
e.g.:
background-size: 100% computes to 100% 100% instead of to 100% auto;
Comment 41•15 years ago
|
||
Last I checked both incorrectly treat b-s:X as b-s:X X rather than b-s:X auto. I filed a WebKit bug on the matter, and presumably they'll be fixing it Real Soon Now as they've removed their -webkit- prefix from the property.
https://bugs.webkit.org/show_bug.cgi?id=28440
No word on Opera as they don't really have a way to give concise technical feedback of this nature.
Comment 42•15 years ago
|
||
Comment 43•15 years ago
|
||
Verified fixed on trunk and 1.9.2 with builds on OS X, Linux, and Windows like:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20090902 Minefield/3.7a1pre ID:20090902030655
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a2pre) Gecko/20090903 Namoroka/3.6a2pre ID:20090903033550
Status: RESOLVED → VERIFIED
Flags: blocking1.9.2? → in-testsuite+
Keywords: verified1.9.2
Target Milestone: --- → mozilla1.9.3a1
You need to log in
before you can comment on or make changes to this bug.
Description
•