Bug 86155 - getComputedStyle for background shorthand property does not return background-origin and background-clip
: getComputedStyle for background shorthand property does not return background...
Status: RESOLVED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
: 86739
:
  Show dependency treegraph
 
Reported: 2012-05-10 16:33 PST by
Modified: 2012-06-05 13:18 PST (History)


Attachments
simple test case (355 bytes, text/html)
2012-05-10 16:33 PST, Joe Thomas
no flags Details
patch (62.31 KB, text/plain)
2012-05-11 01:21 PST, Joe Thomas
no flags Details
patch (81.37 KB, patch)
2012-05-20 08:40 PST, Joe Thomas
no flags Review Patch | Details | Formatted Diff | Diff
patch (59.58 KB, patch)
2012-05-22 00:20 PST, Joe Thomas
no flags Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2012-05-10 16:33:31 PST
Created an attachment (id=141291) [details]
simple test case

background-origin and background-clip are not returned in getComputedStyle for background shorthand property.

The specification related to this is at http://www.w3.org/TR/css3-background/#background.
------- Comment #1 From 2012-05-11 01:21:33 PST -------
Created an attachment (id=141351) [details]
patch
------- Comment #2 From 2012-05-16 15:50:11 PST -------
(From update of attachment 141351 [details])
Why does the position come last?  In the spec, it looks like background-origin and background-clip come last.
http://www.w3.org/TR/css3-background/#background
------- Comment #3 From 2012-05-16 16:25:56 PST -------
(In reply to comment #2)
> (From update of attachment 141351 [details] [details])
> Why does the position come last?  In the spec, it looks like background-origin and background-clip come last.
> http://www.w3.org/TR/css3-background/#background

I think the order is not so strict. Even CSSParser allows the properties to be specified in any order in the background shorthand property. I think it is only strict about background-origin to be specified before background-clip and background-position before background-size. Please correct me if am wrong.
------- Comment #4 From 2012-05-16 16:29:39 PST -------
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 141351 [details] [details] [details])
> > Why does the position come last?  In the spec, it looks like background-origin and background-clip come last.
> > http://www.w3.org/TR/css3-background/#background
> 
> I think the order is not so strict. Even CSSParser allows the properties to be specified in any order in the background shorthand property. I think it is only strict about background-origin to be specified before background-clip and background-position before background-size. Please correct me if am wrong.

That's true, both are something that we can parse correctly.  However, I think we should strive to follow the order that it is spec'ed to better match what people will write.  E.g., borders are often written as "1px solid blue" so getComputedStyle on a border should return the values in that order.

background-origin and background-clip are pretty new.  Do you think that people will likely write it in the order we've chosen?
------- Comment #5 From 2012-05-16 16:41:56 PST -------
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 141351 [details] [details] [details] [details])
> > > Why does the position come last?  In the spec, it looks like background-origin and background-clip come last.
> > > http://www.w3.org/TR/css3-background/#background
> > 
> > I think the order is not so strict. Even CSSParser allows the properties to be specified in any order in the background shorthand property. I think it is only strict about background-origin to be specified before background-clip and background-position before background-size. Please correct me if am wrong.
> 
> That's true, both are something that we can parse correctly.  However, I think we should strive to follow the order that it is spec'ed to better match what people will write.  E.g., borders are often written as "1px solid blue" so getComputedStyle on a border should return the values in that order.
> 
> background-origin and background-clip are pretty new.  Do you think that people will likely write it in the order we've chosen?

Ok. I will make the change. Currently CSSPropertyBackgroundColor is mentioned first, but we need to move it towards the end as per spec. The new order will be

CSSPropertyBackgroundImage,
CSSPropertyBackgroundPosition,
CSSPropertyBackgroundSize,
CSSPropertyBackgroundRepeat, 
CSSPropertyBackgroundAttachment,
CSSPropertyBackgroundOrigin,
CSSPropertyBackgroundClip,
CSSPropertyBackgroundColor,
------- Comment #6 From 2012-05-16 16:58:56 PST -------
(In reply to comment #5)
> Ok. I will make the change. Currently CSSPropertyBackgroundColor is mentioned first, but we need to move it towards the end as per spec. The new order will be
> 
> CSSPropertyBackgroundImage,
> CSSPropertyBackgroundPosition,
> CSSPropertyBackgroundSize,
> CSSPropertyBackgroundRepeat, 
> CSSPropertyBackgroundAttachment,
> CSSPropertyBackgroundOrigin,
> CSSPropertyBackgroundClip,
> CSSPropertyBackgroundColor,

Yeah, that seems better. Thanks!
------- Comment #7 From 2012-05-17 08:36:06 PST -------
Changing the position of background-size from the end to the middle uncovered a bug in CSSParser. The issue is that when the background-size is specified by a single value in background shorthand, parsing is getting failed. Bug 86739 is created for it.
------- Comment #8 From 2012-05-18 12:29:09 PST -------
(From update of attachment 141351 [details])
making the patch obsolete. I will update the patch soon.
------- Comment #9 From 2012-05-20 08:40:01 PST -------
Created an attachment (id=142905) [details]
patch
------- Comment #10 From 2012-05-21 09:23:55 PST -------
(From update of attachment 142905 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=142905&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2695
> +    static const CSSPropertyID properties1[2] = { CSSPropertyBackgroundImage, CSSPropertyBackgroundPosition };
> +
> +    static const CSSPropertyID properties2[6] = { CSSPropertyBackgroundSize, CSSPropertyBackgroundRepeat,

Why do we use 2 arrays here?  Can we name these better than properties1 and properties2 (the variables names may make it more clear why there are 2 arrays here)?
------- Comment #11 From 2012-05-21 09:37:43 PST -------
(In reply to comment #10)
> (From update of attachment 142905 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142905&action=review
> 
> > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2695
> > +    static const CSSPropertyID properties1[2] = { CSSPropertyBackgroundImage, CSSPropertyBackgroundPosition };
> > +
> > +    static const CSSPropertyID properties2[6] = { CSSPropertyBackgroundSize, CSSPropertyBackgroundRepeat,
> 
> Why do we use 2 arrays here?  Can we name these better than properties1 and properties2 (the variables names may make it more clear why there are 2 arrays here)?

For adding the '/' between background-position and background-size properties, I am creating a SlashSeperatedList and appending the lists I created from properties1 array and properties2 array. Actually I could not think of any good names for these arrays, so I just named them properties1 and properties2. Please let me know if there are good names for these arrays.
------- Comment #12 From 2012-05-21 09:41:04 PST -------
(In reply to comment #11)
> For adding the '/' between background-position and background-size properties, I am creating a SlashSeperatedList and appending the lists I created from properties1 array and properties2 array. Actually I could not think of any good names for these arrays, so I just named them properties1 and properties2. Please let me know if there are good names for these arrays.

Can old UAs (i.e. IE6) can parse these results correctly? For editing purposes (and for other apps that store CSS values on server-side), it's very important the shorthand values we generate can be parsed by old UAs.
------- Comment #13 From 2012-05-21 10:37:53 PST -------
View in context: https://bugs.webkit.org/attachment.cgi?id=142905&action=review

>>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2695
>>> +    static const CSSPropertyID properties2[6] = { CSSPropertyBackgroundSize, CSSPropertyBackgroundRepeat,
>> 
> For adding the '/' between background-position and background-size properties, I am creating a SlashSeperatedList and appending the lists I created from properties1 array and properties2 array. Actually I could not think of any good names for these arrays, so I just named them properties1 and properties2. Please let me know if there are good names for these arrays.

How about propertiesBeforeSlash and propertiesAfterSlash or propertiesBeforeSeparator?


(In reply to comment #12)
> 
> Can old UAs (i.e. IE6) can parse these results correctly? For editing purposes (and for other apps that store CSS values on server-side), it's very important the shorthand values we generate can be parsed by old UAs.

I think this should be brought up on a spec mailing list (www-style, I guess).  It's only recently that WebKit started generating shorthand values.  I'm not sure if FireFox or Opera have tried to maintain any backwards compatibility here.
------- Comment #14 From 2012-05-21 10:43:54 PST -------
(In reply to comment #13)
> (In reply to comment #12)
> > Can old UAs (i.e. IE6) can parse these results correctly? For editing purposes (and for other apps that store CSS values on server-side), it's very important the shorthand values we generate can be parsed by old UAs.
> 
> I think this should be brought up on a spec mailing list (www-style, I guess).  It's only recently that WebKit started generating shorthand values.  I'm not sure if FireFox or Opera have tried to maintain any backwards compatibility here.

We absolutely must. If we're not going to maintain the backward compatibility for regular shorthands, then we'll need a separate mechanism to generate backward compatible shorthand notations for editing purposes.
------- Comment #15 From 2012-05-21 12:04:05 PST -------
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > Can old UAs (i.e. IE6) can parse these results correctly? For editing purposes (and for other apps that store CSS values on server-side), it's very important the shorthand values we generate can be parsed by old UAs.
> > 
> > I think this should be brought up on a spec mailing list (www-style, I guess).  It's only recently that WebKit started generating shorthand values.  I'm not sure if FireFox or Opera have tried to maintain any backwards compatibility here.
> 
> We absolutely must. If we're not going to maintain the backward compatibility for regular shorthands, then we'll need a separate mechanism to generate backward compatible shorthand notations for editing purposes.

Webkit started supporting background shorthand in getComputedStyle from Jan 2012 (http://trac.webkit.org/changeset/104169). 

The current order of CSS properties returned by webkit is as per CSS2.1 spec http://www.w3.org/TR/CSS21/colors.html#background-properties. background-position, background-clip and background-origin are not part of background shorthand property in CSS2.1 spec.

Since Webkit already supports all the properties mentioned in background shorthand property in the CSS3 spec (http://www.w3.org/TR/css3-background/#the-background) and considering the fact that webkit started supporting background shorthand in getComputedStyle only recently, it would be better to start generating the background shorthand property as per CSS3 specification. The new order of properties introduced in this patch is as per CSS3 spec.
------- Comment #16 From 2012-05-21 16:31:29 PST -------
Checked the behavior of getComputedStyle for background shorthand property on various browsers

IE 9 and Firefox 12.0 -  return empty.

Opera 11.64  - Order mentioned in CSS2.1 spec followed by three new properties added to the shorthand in CSS3(<bg-size>, <bg-origin>, <bg-clip>).

Chrome 19.0 -  CSS2.1 order

Latest Webkit - CSS2.1 order followed by background-size. background-size is added as part of http://trac.webkit.org/changeset/116645


CSS2.1 order - <'background-color'> || <bg-image> || <repeat-style> || <attachment> || <position>

CSS3 order - <bg-image> || <position> [ / <bg-size> ]? || <repeat-style> || <attachment> || <bg-origin> || <bg-clip> || <'background-color'> 

I wil post a query to www-style mailing list asking for clarification on the order.
------- Comment #17 From 2012-05-21 16:45:09 PST -------
(In reply to comment #15)
> Since Webkit already supports all the properties mentioned in background shorthand property in the CSS3 spec (http://www.w3.org/TR/css3-background/#the-background) and considering the fact that webkit started supporting background shorthand in getComputedStyle only recently, it would be better to start generating the background shorthand property as per CSS3 specification. The new order of properties introduced in this patch is as per CSS3 spec.

Not necessarily. The most important question is other UAs, particularly old ones such as IE6, can parse the generated value or not.

(In reply to comment #16)
> Checked the behavior of getComputedStyle for background shorthand property on various browsers
> 
> IE 9 and Firefox 12.0 -  return empty.
> 
> Opera 11.64  - Order mentioned in CSS2.1 spec followed by three new properties added to the shorthand in CSS3(<bg-size>, <bg-origin>, <bg-clip>).
> 
> Chrome 19.0 -  CSS2.1 order
> 
> Latest Webkit - CSS2.1 order followed by background-size. background-size is added as part of http://trac.webkit.org/changeset/116645

I'm more interested in knowing whether they can parse the generated values or not, and we definitely need to be compatible with IE6 at least. There are mail clients out there that don't even support as CSS as IE6.
------- Comment #18 From 2012-05-21 20:19:30 PST -------
> (In reply to comment #16)
> > Checked the behavior of getComputedStyle for background shorthand property on various browsers
> > 
> > IE 9 and Firefox 12.0 -  return empty.
> > 
> > Opera 11.64  - Order mentioned in CSS2.1 spec followed by three new properties added to the shorthand in CSS3(<bg-size>, <bg-origin>, <bg-clip>).
> > 
> > Chrome 19.0 -  CSS2.1 order
> > 
> > Latest Webkit - CSS2.1 order followed by background-size. background-size is added as part of http://trac.webkit.org/changeset/116645
> 
> I'm more interested in knowing whether they can parse the generated values or not, and we definitely need to be compatible with IE6 at least. There are mail clients out there that don't even support as CSS as IE6.

Tested it on IE 6 on Win XP and here are the observations:

IE-6 parses the background shorthand correctly if we set the newly introduced properties to the background shorthand in CSS3 spec towards the end. This is same as what Opera is doing currently. The order will be

<bg-color> || <bg-image> || <repeat-style> || <attachment> || <position> || / <bg-size> || <bg-origin> || <bg-clip>

IE6 parses the first five properties correctly and ignores the last three. 

Shall we go ahead with this order?
------- Comment #19 From 2012-05-21 20:24:38 PST -------
(In reply to comment #18)
> Tested it on IE 6 on Win XP and here are the observations:
> 
> IE-6 parses the background shorthand correctly if we set the newly introduced properties to the background shorthand in CSS3 spec towards the end. This is same as what Opera is doing currently. The order will be
> 
> <bg-color> || <bg-image> || <repeat-style> || <attachment> || <position> || / <bg-size> || <bg-origin> || <bg-clip>
> 
> IE6 parses the first five properties correctly and ignores the last three. 
> 
> Shall we go ahead with this order?

Yes, please! Backward compatibility is pretty darn important here.
------- Comment #20 From 2012-05-22 00:20:12 PST -------
Created an attachment (id=143209) [details]
patch

Patch with order of properties in background shorthand: <bg-color> || <bg-image> || <repeat-style> || <attachment> || <position> || / <bg-size> || <bg-origin> || <bg-clip>
------- Comment #21 From 2012-05-22 00:29:40 PST -------
(In reply to comment #19)
> (In reply to comment #18)
> > Tested it on IE 6 on Win XP and here are the observations:
> > 
> > IE-6 parses the background shorthand correctly if we set the newly introduced properties to the background shorthand in CSS3 spec towards the end. This is same as what Opera is doing currently. The order will be
> > 
> > <bg-color> || <bg-image> || <repeat-style> || <attachment> || <position> || / <bg-size> || <bg-origin> || <bg-clip>
> > 
> > IE6 parses the first five properties correctly and ignores the last three. 
> > 
> > Shall we go ahead with this order?
> 
> Yes, please! Backward compatibility is pretty darn important here.

Patch Updated. Thanks.
------- Comment #22 From 2012-05-22 10:14:35 PST -------
(In reply to comment #20)
> Created an attachment (id=143209) [details] [details]
> patch
> 
> Patch with order of properties in background shorthand: <bg-color> || <bg-image> || <repeat-style> || <attachment> || <position> || / <bg-size> || <bg-origin> || <bg-clip>

Please provide this feedback to the www-style working group.  I bet we can get the spec changed.
------- Comment #23 From 2012-05-23 11:37:16 PST -------
(In reply to comment #22)
> (In reply to comment #20)
> > Created an attachment (id=143209) [details] [details] [details]
> > patch
> > 
> > Patch with order of properties in background shorthand: <bg-color> || <bg-image> || <repeat-style> || <attachment> || <position> || / <bg-size> || <bg-origin> || <bg-clip>
> 
> Please provide this feedback to the www-style working group.  I bet we can get the spec changed.

Sure. I sent the mail to www-style@w3.org yesterday but it's yet to show up in http://lists.w3.org/Archives/Public/www-style/2012May/. I read that messages from first time posters will take 1-2 days to show up.
------- Comment #24 From 2012-05-31 11:33:49 PST -------
(In reply to comment #22)
> (In reply to comment #20)
> > Created an attachment (id=143209) [details] [details] [details]
> > patch
> > 
> > Patch with order of properties in background shorthand: <bg-color> || <bg-image> || <repeat-style> || <attachment> || <position> || / <bg-size> || <bg-origin> || <bg-clip>
> 
> Please provide this feedback to the www-style working group.  I bet we can get the spec changed.

http://lists.w3.org/Archives/Public/www-style/2012May/1202.html
------- Comment #25 From 2012-05-31 16:10:47 PST -------
Summarizing the discussion on www-style WG.

1/ The order of background properties are not strict in http://www.w3.org/TR/css3-background/#the-background. || implies that one or more of the
alternatives can occur, in any order, as described in
http://www.w3.org/TR/CSS21/about.html#value-defs

2/ If any server / server-side app wants to store the generated result from a CSS3 compliant UA and later feed the result into CSS2.1 compliant UA like IE6, the correct solution would be parse it at the server side and serve only the parts that IE6 understands(http://lists.w3.org/Archives/Public/www-style/2012May/1242.html).
------- Comment #26 From 2012-05-31 22:16:41 PST -------
As per the discussion in the www-style WG(comment #25), the patch looks valid.

Can someone please review it?
------- Comment #27 From 2012-05-31 23:15:06 PST -------
(In reply to comment #25)
> 2/ If any server / server-side app wants to store the generated result from a CSS3 compliant UA and later feed the result into CSS2.1 compliant UA like IE6, the correct solution would be parse it at the server side and serve only the parts that IE6 understands(http://lists.w3.org/Archives/Public/www-style/2012May/1242.html).

I don't think that's a viable option. Anyway, your current patch uses an order that IE6 can parse, so it looks good to me.
------- Comment #28 From 2012-06-01 10:46:04 PST -------
(From update of attachment 143209 [details])
rniwa: Do we use getComputedStyle when serializing HTML that goes into the clipboard?  Are we worried about compat there?
------- Comment #29 From 2012-06-01 11:17:28 PST -------
(In reply to comment #28)
> (From update of attachment 143209 [details] [details])
> rniwa: Do we use getComputedStyle when serializing HTML that goes into the clipboard?  Are we worried about compat there?

We do. See comments #18 - #20. Given that the new format can be parsed by IE6 (trusting Joe's analysis), this change is probably safe to make.

(Btw, I intended to r+ the patch in comment #27. Not sure what happened here :( )
------- Comment #30 From 2012-06-01 11:42:40 PST -------
(In reply to comment #29)
> (In reply to comment #28)
> > (From update of attachment 143209 [details] [details] [details])
> > rniwa: Do we use getComputedStyle when serializing HTML that goes into the clipboard?  Are we worried about compat there?
> 
> We do. See comments #18 - #20. Given that the new format can be parsed by IE6 (trusting Joe's analysis), this change is probably safe to make.

I bet newer IE7 or IE8 (or Firefox, or older Chrome) will fail to parse it.  The CSS spec says that if you come across an unknown token, we should ignore the whole line.  IE6 happens to work by not being spec compliant.

dbaron[1] points out that this is going to be relatively uncommon at first, since only backgrounds with a background-color, background-clip or background-size will contain incompatible CSS.

If this is really a problem, we could have the code for serializing HTML go through a different code path than just calling getComputedStyle from JS.

[1] http://lists.w3.org/Archives/Public/www-style/2012Jun/0020.html
------- Comment #31 From 2012-06-01 12:00:42 PST -------
(From update of attachment 143209 [details])
Clearing flags on attachment: 143209

Committed r119259: <http://trac.webkit.org/changeset/119259>
------- Comment #32 From 2012-06-01 12:00:48 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #33 From 2012-06-01 12:19:40 PST -------
(In reply to comment #30)
> I bet newer IE7 or IE8 (or Firefox, or older Chrome) will fail to parse it.  The CSS spec says that if you come across an unknown token, we should ignore the whole line.  IE6 happens to work by not being spec compliant.

That's annoying :/  Could someone check this?

(In reply to comment #30)
> If this is really a problem, we could have the code for serializing HTML go through a different code path than just calling getComputedStyle from JS.

Yeah, I've started to think we might need that for editing.
------- Comment #34 From 2012-06-01 12:30:38 PST -------
(In reply to comment #33)
> (In reply to comment #30)
> > I bet newer IE7 or IE8 (or Firefox, or older Chrome) will fail to parse it.  The CSS spec says that if you come across an unknown token, we should ignore the whole line.  IE6 happens to work by not being spec compliant.
> 
> That's annoying :/  Could someone check this?
> 
Sure. I will check this.
------- Comment #35 From 2012-06-05 12:09:37 PST -------
(In reply to comment #33)
> (In reply to comment #30)
> > I bet newer IE7 or IE8 (or Firefox, or older Chrome) will fail to parse it.  The CSS spec says that if you come across an unknown token, we should ignore the whole line.  IE6 happens to work by not being spec compliant.
> 
> That's annoying :/  Could someone check this?
> 
IE6, IE7 and IE9 parses the tokens correctly till it encounters an unknown token. It will not drop the whole line. So the order defined in this patch works well with IEs. (did not get a IE8 to test with)

Latest Opera already supports all the properties as per CSS3 spec. It parses all the properties even if they are out-of-order

Firefox and Chrome 19 drops the entire rule if it encounters unknown tokens.
------- Comment #36 From 2012-06-05 13:18:09 PST -------
(In reply to comment #35)
>
> Latest Opera already supports all the properties as per CSS3 spec. It parses all the properties even if they are out-of-order
> 
> Firefox and Chrome 19 drops the entire rule if it encounters unknown tokens.

Thanks for the analysis!  I'm less worried about Firefox here because Gecko isn't embedded (except Thunderbird of course) in many other applications, particularly by mail clients, like IE is.