219829 - mozilla does not handle background-position: <absolute> center

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Tomas Muller]

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-4, no longer working on Mozilla)]

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]

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]

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-4, no longer working on Mozilla)]

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]

Fine :) Just note that I've deleted the example pages since we all know what's
going on.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Is this part of CSS2.1 stable enough that I'd be safe with implementing this?

Comment 7

[Hixie (not reading bugmail)]

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-4, no longer working on Mozilla)]

Well, I still don't like it, but whatever...

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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)]

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 11

[Hixie (not reading bugmail)]

See:
http://www.w3.org/TR/2004/CR-CSS21-20040225/colors.html#propdef-background-position

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

http://www.hixie.ch/tests/adhoc/css/background/position/002.html
http://www.hixie.ch/tests/adhoc/css/background/position/003.html

Comment 13

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

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)]

Attachment: Updated patch

Comment 16

[Hixie (not reading bugmail)]

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-4, no longer working on Mozilla)]

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-4, no longer working on Mozilla)]

(And sorry for the delay.)

Comment 19

[Christopher Aillon (sabbatical, not receiving bugmail)]

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]]

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)]

Checked in 2004-04-09 22:08 PDT.

Comment 22

[Christopher Aillon (sabbatical, not receiving bugmail)]

(this is in for 1.7final)