Skip to content

[css-align] Fix order of unsafe/safe keyword wrt alignment keyword? #1001

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.

Sign up for GitHub

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

Jump to bottom
Closed
fantasai opened this issue Feb 2, 2017 · 3 comments
Closed

[css-align] Fix order of unsafe/safe keyword wrt alignment keyword? #1001

fantasai opened this issue Feb 2, 2017 · 3 comments
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-align-3 Current Work

Comments

@fantasai
Copy link
Collaborator

fantasai commented Feb 2, 2017

Currently align-self and justify-self are defined to allow [ <overflow-position>? && <content-position> ]. That these keywords allow reordering makes parsing ambiguous in the shorthand, so they are disallowed from the shorthand. However, we could fix the order of the keywords, and that would make them unambiguous in the shorthand. It would, however, violate the "allow reordering unless unambiguous" principle for the longhands.
@fantasai fantasai changed the title [css-align] Fix order of unsafe/safe keyword wrt alignment keyword [css-align] Fix order of unsafe/safe keyword wrt alignment keyword? Feb 2, 2017
@tabatkins
Copy link
Member

Proposal: require safe/unsafe to be first, before the alignment keyword.

@fantasai
Copy link
Collaborator Author

Resolved to accept proposal: https://lists.w3.org/Archives/Public/www-style/2017Apr/0027.html

@fantasai fantasai added the Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. label May 2, 2017
fantasai added a commit that referenced this issue Jun 15, 2017
@fantasai
[css-align] Complete changes required by #1001; fixes #1446 and #1445.
0f3e083
Loirooriol pushed a commit to Loirooriol/csswg-drafts that referenced this issue Jun 18, 2017
@fantasai
[css-align] Complete changes required by w3c#1001; fixes w3c#1446 and w…
4160355 
…3c#1445.
@javifernandez
Copy link
Contributor

I've implemented this change for both WebKit and Blink.

https://bugs.chromium.org/p/chromium/issues/detail?id=802098
https://bugs.webkit.org/show_bug.cgi?id=181793

@tabatkins tabatkins mentioned this issue Apr 4, 2018
Closed
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 17, 2018
@javifernandez @chromium-wpt-export-bot
[css-align] Simple syntax for the Alignment shorthands
a0b67ea 
Now that the issue [1] about the syntax ambiguity has bee resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 19, 2018
@javifernandez @chromium-wpt-export-bot
[css-align] Simple syntax for the Alignment shorthands
f419142 
Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 19, 2018
@javifernandez @chromium-wpt-export-bot
[css-align] Simple syntax for the Alignment shorthands
2bccf55 
Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 20, 2018
@javifernandez @chromium-wpt-export-bot
[css-align] Simple syntax for the Alignment shorthands
fdaad68 
Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Reviewed-on: https://chromium-review.googlesource.com/1013710
Commit-Queue: Javier Fernandez <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Manuel Rego Casasnovas <[email protected]>
Cr-Commit-Position: refs/heads/master@{#552293}
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 20, 2018
@javifernandez
[css-align] Simple syntax for the Alignment shorthands
3afa604 
Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.


[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Reviewed-on: https://chromium-review.googlesource.com/1013710
Commit-Queue: Javier Fernandez <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Manuel Rego Casasnovas <[email protected]>
Cr-Commit-Position: refs/heads/master@{#552293}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 20, 2018
@javifernandez @chromium-wpt-export-bot
[css-align] Simple syntax for the Alignment shorthands
4736f3f 
Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Reviewed-on: https://chromium-review.googlesource.com/1013710
Commit-Queue: Javier Fernandez <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Manuel Rego Casasnovas <[email protected]>
Cr-Commit-Position: refs/heads/master@{#552293}
hubot pushed a commit to WebKit/WebKit-http that referenced this issue Apr 20, 2018
Update Alignment shorthands to the spec now that they are not ambiguous
5ae161c 
https://bugs.webkit.org/show_bug.cgi?id=184812

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

Imported new Web Platform Tests from the css-align test suite.
Additionally, updated the ones we already have to verify the new shorthand syntax is correct.

* resources/import-expectations.json:
* web-platform-tests/css/css-align/OWNERS: Added.
* web-platform-tests/css/css-align/content-distribution/place-content-shorthand-001.html:
* web-platform-tests/css/css-align/content-distribution/place-content-shorthand-002.html:
* web-platform-tests/css/css-align/content-distribution/place-content-shorthand-004-expected.txt:
* web-platform-tests/css/css-align/content-distribution/place-content-shorthand-004.html:
* web-platform-tests/css/css-align/default-alignment/parse-justify-items-001.html:
* web-platform-tests/css/css-align/default-alignment/parse-justify-items-003.html:
* web-platform-tests/css/css-align/default-alignment/place-items-shorthand-001.html:
* web-platform-tests/css/css-align/default-alignment/place-items-shorthand-002.html:
* web-platform-tests/css/css-align/default-alignment/place-items-shorthand-004-expected.txt:
* web-platform-tests/css/css-align/default-alignment/place-items-shorthand-004.html:
* web-platform-tests/css/css-align/default-alignment/shorthand-serialization-001-expected.txt: Added.
* web-platform-tests/css/css-align/default-alignment/shorthand-serialization-001.html: Added.
* web-platform-tests/css/css-align/default-alignment/w3c-import.log:
* web-platform-tests/css/css-align/gaps/column-gap-parsing-001-expected.txt:
* web-platform-tests/css/css-align/gaps/column-gap-parsing-001.html:
* web-platform-tests/css/css-align/gaps/gap-normal-computed-001-expected.txt: Added.
* web-platform-tests/css/css-align/gaps/gap-normal-computed-001.html: Added.
* web-platform-tests/css/css-align/gaps/gap-normal-used-001-expected.xht: Added.
* web-platform-tests/css/css-align/gaps/gap-normal-used-001.html: Added.
* web-platform-tests/css/css-align/gaps/gap-normal-used-002-expected.xht: Added.
* web-platform-tests/css/css-align/gaps/gap-normal-used-002.html: Added.
* web-platform-tests/css/css-align/gaps/gap-parsing-001-expected.txt:
* web-platform-tests/css/css-align/gaps/gap-parsing-001.html:
* web-platform-tests/css/css-align/gaps/grid-column-gap-parsing-001-expected.txt:
* web-platform-tests/css/css-align/gaps/grid-column-gap-parsing-001.html:
* web-platform-tests/css/css-align/gaps/grid-gap-parsing-001-expected.txt:
* web-platform-tests/css/css-align/gaps/grid-gap-parsing-001.html:
* web-platform-tests/css/css-align/gaps/grid-row-gap-parsing-001-expected.txt:
* web-platform-tests/css/css-align/gaps/grid-row-gap-parsing-001.html:
* web-platform-tests/css/css-align/gaps/row-gap-parsing-001-expected.txt:
* web-platform-tests/css/css-align/gaps/row-gap-parsing-001.html:
* web-platform-tests/css/css-align/gaps/w3c-import.log:
* web-platform-tests/css/css-align/resources/alignment-parsing-utils.js:
* web-platform-tests/css/css-align/self-alignment/place-self-shorthand-001.html:
* web-platform-tests/css/css-align/self-alignment/place-self-shorthand-002.html:
* web-platform-tests/css/css-align/self-alignment/place-self-shorthand-004-expected.txt:
* web-platform-tests/css/css-align/self-alignment/place-self-shorthand-004.html:
* web-platform-tests/css/css-align/w3c-import.log: Added.

Source/WebCore:

Now that the issue [1] about the syntax ambiguity has been resolved we
don't need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Tests: imported/w3c/web-platform-tests/css/css-align/default-alignment/shorthand-serialization-001.html
       imported/w3c/web-platform-tests/css/css-align/gaps/gap-normal-computed-001.html
       imported/w3c/web-platform-tests/css/css-align/gaps/gap-normal-used-001.html
       imported/w3c/web-platform-tests/css/css-align/gaps/gap-normal-used-002.html

* css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumePlaceContentShorthand): Using the justify-content and align-content parsing logic to parse the shorthand.
(WebCore::CSSPropertyParser::consumePlaceItemsShorthand): Using the justify-items and align-items parsing logic to parse the shorthand.
(WebCore::CSSPropertyParser::consumePlaceSelfShorthand): Using the justify-self and align-self parsing logic to parse the shorthand.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@230848 268f45cc-cd09-0410-ab3c-d52691b4dbfc
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 30, 2018
@javifernandez @moz-wptsync-bot
Bug 1454587 [wpt PR 10502] - [css-align] Simple syntax for the Alignm…
d998717 
…ent shorthands, a=testonly

Automatic update from web-platform-tests[css-align] Simple syntax for the Alignment shorthands

Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Reviewed-on: https://chromium-review.googlesource.com/1013710
Commit-Queue: Javier Fernandez <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Manuel Rego Casasnovas <[email protected]>
Cr-Commit-Position: refs/heads/master@{#552293}

--

wpt-commits: 4736f3fac17173f5e72c307ae586816169873fde
wpt-pr: 10502
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 2, 2019
@marco-c
Bug 1454587 [wpt PR 10502] - [css-align] Simple syntax for the Alignm…
966b8bc 
…ent shorthands, a=testonly

Automatic update from web-platform-tests[css-align] Simple syntax for the Alignment shorthands

Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Reviewed-on: https://chromium-review.googlesource.com/1013710
Commit-Queue: Javier Fernandez <jfernandezigalia.com>
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Reviewed-by: Manuel Rego Casasnovas <regoigalia.com>
Cr-Commit-Position: refs/heads/master{#552293}

--

wpt-commits: 4736f3fac17173f5e72c307ae586816169873fde
wpt-pr: 10502

UltraBlame original commit: fba98c6ad8e53df53c2c92fc8daa5dc15f8935de
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
@marco-c
Bug 1454587 [wpt PR 10502] - [css-align] Simple syntax for the Alignm…
ae91cbe 
…ent shorthands, a=testonly

Automatic update from web-platform-tests[css-align] Simple syntax for the Alignment shorthands

Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Reviewed-on: https://chromium-review.googlesource.com/1013710
Commit-Queue: Javier Fernandez <jfernandezigalia.com>
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Reviewed-by: Manuel Rego Casasnovas <regoigalia.com>
Cr-Commit-Position: refs/heads/master{#552293}

--

wpt-commits: 4736f3fac17173f5e72c307ae586816169873fde
wpt-pr: 10502

UltraBlame original commit: fba98c6ad8e53df53c2c92fc8daa5dc15f8935de
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
@marco-c
Bug 1454587 [wpt PR 10502] - [css-align] Simple syntax for the Alignm…
4433103 
…ent shorthands, a=testonly

Automatic update from web-platform-tests[css-align] Simple syntax for the Alignment shorthands

Now that the issue [1] about the syntax ambiguity has been resolved we
don´t need to use the custom syntax anymore. The Alignment shorthands
use now the simple syntax, defined based on the longhands' syntax.

Since we allow all the values valid for each longhand, we'll update
in this CL the corresponding web platform tests. Additionally, this CL
updates also the shorthand serialization tests [2], which didn't
consider the new value 'legacy' for justify-items (and place-items) due
to the bug [3] Firefox still has pending to be fixed.

[1] w3c/csswg-drafts#1001
[2] css/css-align/default-alignment/shorthand-serialization-001.html
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1363875

Bug: 832503, 829806
Change-Id: I53f803b384cc55b0b38292540262e54f803586da
Reviewed-on: https://chromium-review.googlesource.com/1013710
Commit-Queue: Javier Fernandez <jfernandezigalia.com>
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Reviewed-by: Manuel Rego Casasnovas <regoigalia.com>
Cr-Commit-Position: refs/heads/master{#552293}

--

wpt-commits: 4736f3fac17173f5e72c307ae586816169873fde
wpt-pr: 10502

UltraBlame original commit: fba98c6ad8e53df53c2c92fc8daa5dc15f8935de
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-align-3 Current Work
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tabatkins @fantasai @javifernandez