'center right' and 'center left' 'background-position' doesn't parse within shorthand

RESOLVED FIXED in mozilla0.9.9

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Eric A. Meyer (dead account), Assigned: bz)

Tracking

Trunk
mozilla0.9.9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

15 years ago
You can see the problem in action on http://www.meyerweb.com/ by selecting the
presentation choice "natural" and allowing the cookie to be set.  (If you want
to delete it after testing, the name is 'meyerweb-style'.)  The site's
"masthead" doesn't take on the green background and butterfly image that it
should.  The problem seems to center around styling the same element multiple
times using different stylesheets.  I will attach a reduced testcase
demonstrating that this is (apparently) confined to background styles and not
others.
(Reporter)

Comment 1

15 years ago
Created attachment 68339 [details]
Reduced testcase (based on meyerweb.com)

Note that the font-family and color are set by the second stylesheet in the
source, just as they should be, but the background styles are ignored.	This
happens on meyerweb.com with LINKed external stylesheets, and here the
stylesheets are embedded in the HEAD element.  The behavior is the same either
way.
I looked at the code and I didn't see anything obviously wrong with
'background-color' except that we don't do 'background-color: inherit' properly
-- we treat it tas transparent.

Is this only a problem with 'background-color', or other things?  (I can't look
at the testcase right now...)
This is a parsing problem.  We're ignoring that 'background' line for some
reason.  If I take out the 'center right' it works.
I filed bug 124219 on the other issue I noticed.
The problem seems to be that 'center right' and 'center left' values for
'background-position' don't seem to parse within the shorthand.  'center top'
and 'center bottom' seem to work OK.  I didn't try very much else.
Summary: Backgrounds not cascading properly → 'center right' and 'center left' 'background-position' doesn't parse within shorthand
Created attachment 68359 [details]
simpler testcase
Attachment #68339 - Attachment is obsolete: true
(Reporter)

Comment 7

15 years ago
Verifying that 'center top' and 'center bottom' are fine.  I also tried 'left
center' and 'right center' and they were okay.  It would seem that any time
'center' comes before the either one of the keywords 'right' or 'left', Mozilla
gets confused.
(Reporter)

Comment 8

15 years ago
Created attachment 68369 [details]
Stepping through background-position keyword combinations
Attachment #68359 - Attachment is obsolete: true
(Reporter)

Comment 9

15 years ago
Created attachment 68371 [details]
Stepping through background-position keyword combinations

This testcase shows all of the various combinations for position keywords. 
Green backgrounds showing an image are okay.  Red backgrounds with no image
fail.
(Reporter)

Updated

15 years ago
Attachment #68369 - Attachment is obsolete: true
We have eCSSProperty_background_x_position coming before
eCSSProperty_background_y_position in the kBackgroundIDs list in ParseBackground.

So when parsing this shorthand we try the x-position parser on "center" and it
works.  So we say that x-position is center.  Then we try to parse "right" and
parse y-position as "right".

Now we go back to ParseBackground and start validating the values.  x-position
is "really an X value", so we check the y value which is _not_ really a y-value,
and then we fail.

The validation logic needs to handle this case.
Created attachment 68377 [details] [diff] [review]
Proposed patch
Keywords: patch, review

Comment 12

15 years ago
Comment on attachment 68377 [details] [diff] [review]
Proposed patch

r=pierre
Attachment #68377 - Flags: review+

Comment 13

15 years ago
Comment on attachment 68377 [details] [diff] [review]
Proposed patch

sr=attinasi
Attachment #68377 - Flags: superreview+
a=roc+moz for 0.9.9
Keywords: mozilla0.9.9+
->bzbarsky, so he doesn't forget to land it :-)
Assignee: dbaron → bzbarsky
Target Milestone: --- → mozilla0.9.9
(Reporter)

Comment 16

15 years ago
I assume the patch will also fix this problem in 'background-position' , 
which suffer the same problem?  The patch appears to do so but I wanted 
to make double-sure.
Checked in on trunk (pre-branch).
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
erm, actually I'm pretty sure this does not fix any problems there may be with
background-position.  Eric, could you file a bug on me on that?  (if you have a
testcase that would be great too).
Just looked at the code for background-position.  It's correct.  I also modified
the testcase in this bug to use background-position and we passed.  So no new
bug needed that I can see.  :)
You need to log in before you can comment on or make changes to this bug.