Last Comment Bug 219829 - mozilla does not handle background-position: <absolute> center
: mozilla does not handle background-position: <absolute> center
Status: RESOLVED FIXED
[css2.1]
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla1.7final
Assigned To: Christopher Aillon (sabbatical, not receiving bugmail)
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-09-20 18:59 PDT by Tomas Muller
Modified: 2004-04-09 22:37 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (12.27 KB, patch)
2004-03-08 12:12 PST, Christopher Aillon (sabbatical, not receiving bugmail)
no flags Details | Diff | Review
Updated patch (12.27 KB, patch)
2004-03-09 15:37 PST, Christopher Aillon (sabbatical, not receiving bugmail)
dbaron: review+
dbaron: superreview+
asa: approval1.7+
Details | Diff | Review

Description Tomas Muller 2003-09-20 18:59:01 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20030916
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20030916

Look at the URL demonstrating the problem. I was using ver 1.4b at first, but
when I read "Also, please do not file bugs on copies of Mozilla older than two
weeks." I downloaded the new rc of 1.5 and the problem is just the same. See
http://sklad.hujer.cz/mozbug/azlan/css/index.css for the css definition:
ul.whatsUp li. Please note that this is a bussiness page; I should not be
letting it out yet. I just don't have time at the moment to play around with
this bug :-/ The background- properties are split up here, if I defined them as
a shorthand backround: ... property, i have had problems with Opera7 and IE6
displaying the backround differently - horizontally centered and
12(whatever)pixels from the top and vice versa. Mozilla didn't display anything.

Reproducible: Didn't try

Steps to Reproduce:
1.
2.
3.
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-09-20 19:15:48 PDT
absolute lengths could not be mixed with keywords in CSS2.  I think they can in
CSS2.1, but that's a very recent decision and hasn't been implemented yet.
Comment 2 Tomas Muller 2003-09-20 19:23:09 PDT
Well you're right, I did not pay enough attention reading the specs. Sorry. I
just put 50% instead of 'center' keyword and it works fine.
Comment 3 Tomas Muller 2003-09-20 21:18:09 PDT
Therefore I resolve this "bug" changing its resolution to INVALID. But I think
that the spec-makers could allow this. They go like 'center' is the same as
'50%' and then people like me get confused :)
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-09-20 22:08:43 PDT
Reopening.  It is allowed by a revision of the spec that came out on Monday (but
is currently still a draft).  But we haven't exactly had a chance to implement
that yet.  However, this bug would be a good reminder to do so.
Comment 5 Tomas Muller 2003-09-20 22:22:37 PDT
Fine :) Just note that I've deleted the example pages since we all know what's
going on.
Comment 6 Boris Zbarsky [:bz] 2003-11-16 00:36:12 PST
Is this part of CSS2.1 stable enough that I'd be safe with implementing this?
Comment 7 Hixie (not reading bugmail) 2003-11-16 04:58:20 PST
I would guess yes. We discussed this at some length to get the current concensus
and I don't recall anyone raising an issue on it since then.
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-11-16 10:39:44 PST
Well, I still don't like it, but whatever...
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-11-16 10:42:54 PST
Actually, never mind.  I was thinking of something else.  Go ahead...

But note that the grammar is wrong and the prose is somewhat unclear.  ("If two
values are given" means "percentage or length values".)
Comment 10 Hixie (not reading bugmail) 2003-11-16 11:18:16 PST
Grammar issue noted as issue 228. I disagree with the prose issue, since the 
paragraph starts "If only one percentage or length value is given, it sets the
horizontal position only, and the vertical position will be 50%. If two values
are given, the horizontal position comes first." so IMHO it is clear that "two
values" still refers to "two percentage or length values".
Comment 13 Christopher Aillon (sabbatical, not receiving bugmail) 2004-03-08 12:12:05 PST
Created attachment 143314 [details] [diff] [review]
Patch
Comment 14 Christopher Aillon (sabbatical, not receiving bugmail) 2004-03-09 14:55:01 PST
Comment on attachment 143314 [details] [diff] [review]
Patch

>+      PRInt32 xValue = values[4].GetIntValue();
>+      PRInt32 yValue = values[5].GetIntValue();
>+      if (eCSSUnit_Enumerated == xUnit) {
>         if (eCSSUnit_Enumerated == yUnit) {
>-          PRInt32 xValue = values[4].GetIntValue();
>-          PRInt32 yValue = values[5].GetIntValue();

I changed that to:

>+      if (eCSSUnit_Enumerated == xUnit) {
>+        PRInt32 xValue = values[4].GetIntValue();
>         if (eCSSUnit_Enumerated == yUnit) {
>           PRInt32 yValue = values[5].GetIntValue();

and added a local declaration of yValue in the |if (eCSSUnit_Enumerated ==
yUnit)| clause contained by the else portion of |if (eCSSUnit_Enumerated == xUnit)|.
Comment 15 Christopher Aillon (sabbatical, not receiving bugmail) 2004-03-09 15:37:34 PST
Created attachment 143446 [details] [diff] [review]
Updated patch
Comment 16 Hixie (not reading bugmail) 2004-03-10 16:42:43 PST
caillon: the issue you raised (what if only one keyword is given) is actually
already covered by the spec in the list of possible value combinations before
the paragraph you mentioned.
Comment 17 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-03-29 18:04:29 PST
Comment on attachment 143446 [details] [diff] [review]
Updated patch

>   if (0 != (found & 0x30)) {  // found one or more position values, validate them
>     if (0 == (found & 0x20)) {  // x value only

This comment ("x value only") isn't correct (and never was).

>       else if (eCSSUnit_Inherit == values[4].GetUnit()) {
>         values[5].SetInheritValue();
>       }
>       else if (eCSSUnit_Initial == values[4].GetUnit()) {
>         values[5].SetInitialValue();
>       }

These are unnecessary.

r+sr=dbaron
Comment 18 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-03-29 18:29:56 PST
(And sorry for the delay.)
Comment 19 Christopher Aillon (sabbatical, not receiving bugmail) 2004-03-30 14:38:10 PST
Comment on attachment 143446 [details] [diff] [review]
Updated patch

dbaron suggested this would be good to land in 1.7, so seeking approval.
Comment 20 Asa Dotzler [:asa] 2004-04-07 15:34:49 PDT
Comment on attachment 143446 [details] [diff] [review]
Updated patch

a=asa (on behalf of drivers) for checkin to 1.7
Comment 21 Christopher Aillon (sabbatical, not receiving bugmail) 2004-04-09 22:20:53 PDT
Checked in 2004-04-09 22:08 PDT.
Comment 22 Christopher Aillon (sabbatical, not receiving bugmail) 2004-04-09 22:37:03 PDT
(this is in for 1.7final)

Note You need to log in before you can comment on or make changes to this bug.