Closed Bug 1329381 Opened 7 years ago Closed 7 years ago

Computed value of background-repeat should be a list, each item consisting of: two keywords, one per dimension.

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: kuoe0.tw, Assigned: kuoe0.tw)

Details

Attachments

(2 files)

Bug 1329381 - (Part 1) Computed value of background-repeat should be a list, each item consisting of: two keywords, one per dimension.
59 bytes, text/x-review-board-request
Details
Bug 1329381 - (Part 2) Use the same format like the computed value as the speicified value for background-repeat.
59 bytes, text/x-review-board-request
Details 
In the spec of "Backgrounds and Borders Module Level 3"[1], the computed value of background-repeat should be a list. And each item consists of two keywords, one per dimension. In current implementation, we followed the old spec[2], use the specified value as the computed value.

[1]: https://www.w3.org/TR/css3-background/#background-repeat
[2]: https://www.w3.org/TR/CSS2/colors.html#propdef-background-repeat
Current state:

Specified value  | Computed value
-----------------|-----------------
repeat           | repeat
repeat-x         | repeat-x
repeat-y         | repeat-y
repeat repeat    | repeat
repeat no-repeat | repeat-x
no-repeat repeat | repeat-y
space no-repeat  | space no-repeat
round repeat     | round repeat

It even doesn't follow the spec of level 2. In CSS level 2, the computed value should be "as specified", so the computed value of "repeat no-repeat" should be "repeat no-repeat" not "repeat-x". In this bug, we should fix it to be the following state:


Specified value  | Computed value
-----------------|-----------------
repeat repeat    | repeat repeat
repeat-x         | repeat no-repeat
repeat-y         | no-repeat repeat
repeat repeat    | repeat repeat
repeat no-repeat | repeat no-repeat
no-repeat repeat | no-repeat repeat
space no-repeat  | space no-repeat
round repeat     | round repeat
Unfortunately, we don't very strict requirements on how to serialize computed values in CSS specs.

But there are a few general rules that we follow in Gecko, if nothing in the spec tells us how to serialize a computed value:

* If it's a longhand property, we should follow what the Computed Value line in the spec says.

* However, if it is a longhand property that gained new values, we should try to serialize the old values the same way that we used to.

* If it's a shorthand property, we should return "".

* However, if it is a shorthand property that used to be a longhand property (as background-repeat is), then we should try to serialize it like used to when it was a longhand.

The two "However" points are to avoid potential web compatibility problems.  For background-repeat, we might have content that is doing something like |getComputedStyle(element).backgroundRepeat == "repeat-x"|, which would break if we start returning "repeat no-repeat".

For new values, like "round", we are free to follow the general rule.

David mentions the "serialize like we used to" rule in https://lists.w3.org/Archives/Public/www-style/2012Feb/1094.html, but I don't think it's written down in any spec.  (In theory, CSSOM should define exactly what gets returned from getComputedStyle(...)[xxx], but it doesn't include this rule.)


Following those rules make me think we should serialize like this:

Specified value  | Computed value
-----------------|-----------------
repeat           | repeat
repeat-x         | repeat-x
repeat-y         | repeat-y
repeat repeat    | repeat
repeat no-repeat | repeat no-repeat
no-repeat repeat | no-repeat repeat
space no-repeat  | space no-repeat
round repeat     | round repeat
space            | space space
round            | round round

That matches our current behavior, except for "space" and "repeat" which we currently serialize as "space" and "repeat".

But, checking other browsers, Edge, Chrome and Safari all serialize "space" as "space", and not "space space".  This makes me think we shouldn't change our current serialization behavior, and really we should specify this serialization in the spec.

David, does that sound right to you?  What's the current appetite in the CSSWG for standardizing serialization things like this?
Flags: needinfo?(dbaron)
(In reply to Cameron McCormack (:heycam) from comment #6)
> Unfortunately, we don't very strict requirements on how to serialize
> computed values in CSS specs.
> 
> But there are a few general rules that we follow in Gecko, if nothing in the
> spec tells us how to serialize a computed value:
> 
> * If it's a longhand property, we should follow what the Computed Value line
> in the spec says.

I actually don't think this is a general principle.  I think some cssom spec editors tried to do something like that, but I pushed back against specifying things that might go against current interoperable behavior (and potentially breaking interoperability) just for the convenience of specification editors.

But there is the general principle that we should try to serialize to the shortest form when possible.  That's expressed for shorthands in https://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSS2Properties but I think it applies to complex longhands as well.

> * However, if it is a longhand property that gained new values, we should
> try to serialize the old values the same way that we used to.
> 
> * If it's a shorthand property, we should return "".

The spec says we should actually implement these, I think.  There's an open bug on it, bug 137688.

> * However, if it is a shorthand property that used to be a longhand property
> (as background-repeat is), then we should try to serialize it like used to
> when it was a longhand.
> 
> The two "However" points are to avoid potential web compatibility problems. 
> For background-repeat, we might have content that is doing something like
> |getComputedStyle(element).backgroundRepeat == "repeat-x"|, which would
> break if we start returning "repeat no-repeat".
> 
> For new values, like "round", we are free to follow the general rule.
> 
> David mentions the "serialize like we used to" rule in
> https://lists.w3.org/Archives/Public/www-style/2012Feb/1094.html, but I
> don't think it's written down in any spec.  (In theory, CSSOM should define
> exactly what gets returned from getComputedStyle(...)[xxx], but it doesn't
> include this rule.)
> 
> 
> Following those rules make me think we should serialize like this:
> 
> Specified value  | Computed value
> -----------------|-----------------
> repeat           | repeat
> repeat-x         | repeat-x
> repeat-y         | repeat-y
> repeat repeat    | repeat
> repeat no-repeat | repeat no-repeat
> no-repeat repeat | no-repeat repeat
> space no-repeat  | space no-repeat
> round repeat     | round repeat
> space            | space space
> round            | round round
> 
> That matches our current behavior, except for "space" and "repeat" which we
> currently serialize as "space" and "repeat".
> 
> But, checking other browsers, Edge, Chrome and Safari all serialize "space"
> as "space", and not "space space".  This makes me think we shouldn't change
> our current serialization behavior, and really we should specify this
> serialization in the spec.

I think I prefer going with the shorter form.

> David, does that sound right to you?  What's the current appetite in the
> CSSWG for standardizing serialization things like this?

I think there's a bit of willingness to do this.  (For a while we had a "Canonical order:" in propdef tables, although I think only for shorthands, and not consistently between specs.)  It probably should go in the definition of the property in css-backgrounds.
Flags: needinfo?(dbaron)
Attachment #8825273 - Flags: review?(cam)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Comment on attachment 8825274 [details]
Bug 1329381 - (Part 2) Use the same format like the computed value as the speicified value for background-repeat.

https://reviewboard.mozilla.org/r/103448/#review105052
Attachment #8825274 - Flags: review?(boris.chiou)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: