Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-values] Retiring support for 3-valued positions (excluding background) #2140

Closed
ewilligers opened this issue Dec 28, 2017 · 10 comments
Closed
Labels
css-values-4 Current Work

Comments

@ewilligers
Copy link
Contributor

The CSS <position> spec changed early this year, so that 3-valued positions (left 10% top) would no longer be part of <position> syntax, for any properties other than background-position.

The change was accompanied by a motive:

Note: The background-position property also accepts a three-value syntax. This has been disallowed generically because it creates parsing ambiguities when combined with other length or percentage components in a property value.

I added Blink use counters at the time, and we now have data.

ThreeValuedPositionBackground >1%

ThreeValuedPositionBasicShape 0.00004% (spec)

ThreeValuedPositionGradient 0.001% (spec)

ThreeValuedPositionObjectPosition 0.00007% (spec)

ThreeValuedPositionPerspectiveOrigin 0.000001% (spec)

All of the main browser engines still support 3-valued positions as values (apart from Edge for perspective-origin, but it doesn't accept 4-valued perspective-origin values either).

Does the Working Group ratify the retiring of 3-valued positions (excluding background)? Are browser vendors in agreement to remove support?

@Nadya678
Copy link

What is added value in this part of draft? Is there possibility to add any illustration to the draft that is impossible to style with standard styles?

@fantasai
Copy link
Collaborator

fantasai commented Jan 5, 2018

Question: Does the Blink use counter exclude 3-value syntaxes that use the 'center' keyword (since that doesn't take an offset)?

@ewilligers
Copy link
Contributor Author

The Blink use counter excludes syntaxes that have an offset immediately after "center". The counter is only for 3-value syntaxes that match the <bg-position> syntax,

[ center | [ left | right ] <length-percentage>? ] &&
[ center | [ top | bottom ] <length-percentage>? ]

The logic is essentially as follows: The first value is an identifier. Exactly one of the next two values is an identifier, the other is a <length-percentage>. If the second value is an identifier, it is not "center". Otherwise, the first value is not "center". "left" | "right" appear at most once. "top" | "bottom" appear at most once.

@fantasai
Copy link
Collaborator

The Blink use counter excludes syntaxes that have an offset immediately after "center". The counter is only for 3-value syntaxes that match the syntax

Right, I'm pointing out that 3-value syntaxes that use 'center' as one of the keywords (e.g. center top 3px) are still valid with the change -- it's the 3-value syntax without center (e.g. top left 3px) that would become invalid.

@ewilligers
Copy link
Contributor Author

The syntax for position has limited alternatives:

<position> = [
  [ left | center | right ] || [ top | center | bottom ]
|

1 or 2 values

  [ left | center | right | <length-percentage> ]
  [ top | center | bottom | <length-percentage> ]?
|

1 or 2 values

  [ [ left | right ] <length-percentage> ] &&
  [ [ top | bottom ] <length-percentage> ]
]

4 values

Right, I'm pointing out that 3-value syntaxes that use 'center' as one of the keywords (e.g. center top 3px) are still valid with the change

How is that still valid?

@fantasai
Copy link
Collaborator

The current spec for this is https://drafts.csswg.org/css-backgrounds-3/#the-background-position which includes

<bg-position> = [  [ left | center | right | top | bottom | <length-percentage> ]
|
  [ left | center | right | <length-percentage> ]
  [ top | center | bottom | <length-percentage> ]
|
  [ center | [ left | right ] <length-percentage>? ] &&
  [ center | [ top | bottom ] <length-percentage>? ]
]

@css-meeting-bot
Copy link
Member

The Working Group just discussed [css-values] Retiring support for 3-valued positions (excluding background).

The full IRC log of that discussion <dael> topic: [css-values] Retiring support for 3-valued positions (excluding background)
<dael> github: https://github.com//issues/2140
<dael> fantasai: I believe the radial gradient sntax for elipsis allows for just one...not, requires 2.
<dael> fantasai: Nevermind.
<dael> fantasai: There's an issue from ericwilligers with some data about the incidence of 3 value syntax and checking if we want to go ahead with that.
<dael> ericwilligers: We made a spec change months ago, but I don't think any impl changed. Do we still support the spec change and are impl planning to change if so? I don't think one impl will change if the others aren't good to change.
<dael> Rossen_: You're saying based on your data this is a safe change?
<dael> ericwilligers: Yes
<dael> Rossen_: Current proposal is that for basic shapes, gradient, and prespective origin to remove support for 3 value positions is safe and we should do it.
<dael> ericwilligers: Original motive was so we can have % adjacent to positions. That's the motivation since it was ambig. If you have a position followed by a length you wouldn't know what it was.
<dael> fantasai: Yes, the issue was we wanted to use position in more places.
<fantasai> Like transforms
<dael> Rossen_: What are impl opinions? Sounds like ericwilligers/Blink is willing to adopt the change. I would have to check what that means for our (Microsoft) logic and if that would disrupt some of the uses we have of posiions. If this will hurt more then help. But I don't believe this will be the case so I don't object to going ahead with this.
<dael> Rossen_: I'd like to hear from webkit and gecko.
<dael> smfr: I don't have a good feel for web compat risk so I won't know.
<dael> Rossen_: Is this something you can gather that will be more then the data posted by ericwilligers ? Or will you rely on ericwilligers ?
<dael> smfr: We don't have data gathering as fine-grain as ericwilligers 's data.
<dael> smfr: WE'd be willing to change as long as we don't find internal content that would break.
<dael> Rossen_: Gecko?
<dael> dbaron: We'd be fine to change if other engines are willing.
<dael> Rossen_: So ericwilligers it sounds like more or less everyone is willing to change. Is this something you're looking for in terms of time frame? Is it on your schedule?
<dael> ericwilligers: This is fine. I consider that intent to change. Before I put it before the community I wanted to check position, but everything is fine. I'll send out intent this week.
<dael> Rossen_: Anything more on this?
<dael> ericwilligers: No.
<dael> Rossen_: Doesn't sound we need a resolution because we have one to change. Now it's mostly an agreement of when to roll out the change. Thanks.

@astearns astearns removed the Agenda+ label Jan 23, 2018
@gsnedders gsnedders added the css-values-4 Current Work label Jan 24, 2018
@ewilligers
Copy link
Contributor Author

Web platform tests have been added, e.g. https://wpt.fyi/results/css/css-images/parsing/object-position-invalid.html

Blink retires support in Chrome 68.

@BorisChiou
Copy link
Contributor

BorisChiou commented Jul 2, 2019

I just filed an issue for Chrome because it seems mask-position also uses <position>, but Chrome doesn't retire its 3-valued syntax.
https://bugs.chromium.org/p/chromium/issues/detail?id=980731&can=2&q=mask-position

@BorisChiou
Copy link
Contributor

BorisChiou commented Jul 3, 2019

Sorry, my bad. Chrome doesn't support mask-position, so this Chromium issue should be invalid. Thanks for the feedback in that issue. Firefox will retire the 3-valued syntax soon. Thx.

ewilligers pushed a commit to ewilligers/web-platform-tests that referenced this issue Jul 3, 2019
ewilligers added a commit to web-platform-tests/wpt that referenced this issue Jul 4, 2019
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 10, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276
gecko-commit: 6ec2809da4e31720cbb59b4ba71616c1c168553e
gecko-integration-branch: autoland
gecko-reviewers: emilio
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 11, 2019
…tion. r=emilio

According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126

--HG--
extra : moz-landing-system : lando
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jul 11, 2019
…tion. r=emilio

According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 11, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276
gecko-commit: 6ec2809da4e31720cbb59b4ba71616c1c168553e
gecko-integration-branch: autoland
gecko-reviewers: emilio
emilio pushed a commit to emilio/servo that referenced this issue Jul 22, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126
emilio pushed a commit to emilio/servo that referenced this issue Jul 23, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126
emilio pushed a commit to emilio/servo that referenced this issue Jul 23, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126
emilio pushed a commit to emilio/servo that referenced this issue Jul 23, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126
emilio pushed a commit to emilio/servo that referenced this issue Jul 23, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 24, 2019
…estonly

Automatic update from web-platform-tests
[css-masking] Parsing mask-position (#17623)

mask-position accepts <position>#
https://drafts.fxtf.org/css-masking-1/#the-mask-position

3-valued positions are not accepted
w3c/csswg-drafts#2140
--

wpt-commits: 2831268f1ea8bf7fdb7b27109f2cb887bbae6b56
wpt-pr: 17623
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jul 25, 2019
…estonly

Automatic update from web-platform-tests
[css-masking] Parsing mask-position (#17623)

mask-position accepts <position>#
https://drafts.fxtf.org/css-masking-1/#the-mask-position

3-valued positions are not accepted
w3c/csswg-drafts#2140
--

wpt-commits: 2831268f1ea8bf7fdb7b27109f2cb887bbae6b56
wpt-pr: 17623
natechapin pushed a commit to natechapin/wpt that referenced this issue Aug 23, 2019
natechapin pushed a commit to natechapin/wpt that referenced this issue Aug 23, 2019
According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276
gecko-commit: 6ec2809da4e31720cbb59b4ba71616c1c168553e
gecko-integration-branch: autoland
gecko-reviewers: emilio
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…tion. r=emilio

According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126

UltraBlame original commit: 6ec2809da4e31720cbb59b4ba71616c1c168553e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…estonly

Automatic update from web-platform-tests
[css-masking] Parsing mask-position (#17623)

mask-position accepts <position>#
https://drafts.fxtf.org/css-masking-1/#the-mask-position

3-valued positions are not accepted
w3c/csswg-drafts#2140
--

wpt-commits: 2831268f1ea8bf7fdb7b27109f2cb887bbae6b56
wpt-pr: 17623

UltraBlame original commit: 2f1f411688847eef0ad1ba01855022ba1b8b2ec0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…tion. r=emilio

According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126

UltraBlame original commit: 6ec2809da4e31720cbb59b4ba71616c1c168553e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…estonly

Automatic update from web-platform-tests
[css-masking] Parsing mask-position (#17623)

mask-position accepts <position>#
https://drafts.fxtf.org/css-masking-1/#the-mask-position

3-valued positions are not accepted
w3c/csswg-drafts#2140
--

wpt-commits: 2831268f1ea8bf7fdb7b27109f2cb887bbae6b56
wpt-pr: 17623

UltraBlame original commit: 2f1f411688847eef0ad1ba01855022ba1b8b2ec0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…tion. r=emilio

According to this resolved spec issue:
w3c/csswg-drafts#2140,
we retire the 3-valued <position> on
1. `object-position`
2. `perspective-origin`,
3. `mask-position`
4. `circle()` and `ellipse()`
, but still keep the support for `background-position`.

Besides, I simply run this python script to generate the .ini file:
```
s = sys.argv[1] + ".ini"
with open(s, "w") as f:
    f.write('[{}]\n'.format(sys.argv[1]))
    f.write('  expected: FAIL\n')
    f.write('  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n')
```

Differential Revision: https://phabricator.services.mozilla.com/D37126

UltraBlame original commit: 6ec2809da4e31720cbb59b4ba71616c1c168553e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…estonly

Automatic update from web-platform-tests
[css-masking] Parsing mask-position (#17623)

mask-position accepts <position>#
https://drafts.fxtf.org/css-masking-1/#the-mask-position

3-valued positions are not accepted
w3c/csswg-drafts#2140
--

wpt-commits: 2831268f1ea8bf7fdb7b27109f2cb887bbae6b56
wpt-pr: 17623

UltraBlame original commit: 2f1f411688847eef0ad1ba01855022ba1b8b2ec0
yury-s pushed a commit to yury-s/webkit-http that referenced this issue Oct 29, 2019
https://bugs.webkit.org/show_bug.cgi?id=189142
LayoutTests/imported/w3c:

<rdar://problem/44110851>

Reviewed by Antti Koivisto.

New PASS results.

* web-platform-tests/css/css-images/parsing/gradient-position-invalid-expected.txt:
* web-platform-tests/css/css-images/parsing/object-position-invalid-expected.txt:
* web-platform-tests/css/css-shapes/parsing/shape-outside-invalid-position-expected.txt:

Source/WebCore:

Reviewed by Antti Koivisto.

The resolution in w3c/csswg-drafts#2140 changed the syntax for <position>,
disallowing the 3-value syntax. This is used in object-position, gradients and shapes. background-position
continues to use the old syntax.

Fix CSS parsing accordingly.

Tested by css-images WPT, by shapes tests, and object-position tests.

* css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumePerspectiveOrigin):
(WebCore::consumeBasicShapeCircle):
(WebCore::consumeBasicShapeEllipse):
(WebCore::CSSPropertyParser::parseSingleValue):
(WebCore::consumeBackgroundPosition):
(WebCore::CSSPropertyParser::consumeBackgroundShorthand):
* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::backgroundPositionFromThreeValues):
(WebCore::CSSPropertyParserHelpers::positionFromFourValues):
(WebCore::CSSPropertyParserHelpers::consumePosition):
(WebCore::CSSPropertyParserHelpers::consumeRadialGradient):
(WebCore::CSSPropertyParserHelpers::consumeConicGradient):
(WebCore::CSSPropertyParserHelpers::positionFromThreeOrFourValues): Deleted.
* css/parser/CSSPropertyParserHelpers.h:

LayoutTests:

Reviewed by Antti Koivisto.

Land some FAIL results for these shapes tests. They should get removed when
the css/css-shapes WPT are imported (webkit.org/b/203441), though the WPT
haven't been updated for the new syntax either.

* css3/shapes/shape-outside/values/shape-outside-circle-002-expected.txt:
* css3/shapes/shape-outside/values/shape-outside-circle-004-expected.txt:
* css3/shapes/shape-outside/values/shape-outside-ellipse-002-expected.txt:
* css3/shapes/shape-outside/values/shape-outside-ellipse-004-expected.txt:
* fast/css/object-position/parsing-object-position-expected.txt:
* fast/css/object-position/parsing-object-position.html: Remove the invalid position test.
* fast/shapes/parsing/parsing-shape-outside-expected.txt:
* fast/shapes/parsing/parsing-test-utils.js:  Remove the invalid position tests.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251668 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-values-4 Current Work
Projects
None yet
Development

No branches or pull requests

8 participants