Closed
Bug 124193
Opened 23 years ago
Closed 23 years ago
'center right' and 'center left' 'background-position' doesn't parse within shorthand
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla0.9.9
People
(Reporter: emeyer, Assigned: bzbarsky)
References
()
Details
Attachments
(2 files, 3 obsolete files)
1.80 KB,
text/html
|
Details | |
1.58 KB,
patch
|
pierre
:
review+
attinasi
:
superreview+
|
Details | Diff | Splinter Review |
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•23 years ago
|
||
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
Attachment #68339 -
Attachment is obsolete: true
Reporter | ||
Comment 7•23 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•23 years ago
|
||
Attachment #68359 -
Attachment is obsolete: true
Reporter | ||
Comment 9•23 years ago
|
||
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•23 years ago
|
Attachment #68369 -
Attachment is obsolete: true
Assignee | ||
Comment 10•23 years ago
|
||
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.
Assignee | ||
Comment 11•23 years ago
|
||
Assignee | ||
Updated•23 years ago
|
Comment 12•23 years ago
|
||
Comment on attachment 68377 [details] [diff] [review] Proposed patch r=pierre
Attachment #68377 -
Flags: review+
Comment 13•23 years ago
|
||
Comment on attachment 68377 [details] [diff] [review] Proposed patch sr=attinasi
Attachment #68377 -
Flags: superreview+
a=roc+moz for 0.9.9
Updated•23 years ago
|
Keywords: mozilla0.9.9+
->bzbarsky, so he doesn't forget to land it :-)
Assignee: dbaron → bzbarsky
Target Milestone: --- → mozilla0.9.9
Reporter | ||
Comment 16•23 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.
Assignee | ||
Comment 17•23 years ago
|
||
Checked in on trunk (pre-branch).
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 18•23 years ago
|
||
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).
Assignee | ||
Comment 19•23 years ago
|
||
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.
Description
•