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-align] Fix order of `unsafe`/`safe` keyword wrt alignment keyword? #1001

Closed
fantasai opened this Issue Feb 2, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@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

This comment has been minimized.

Copy link
Member

tabatkins commented Mar 14, 2017

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

@fantasai

This comment has been minimized.

Copy link
Collaborator Author

fantasai commented Apr 19, 2017

@javifernandez

This comment has been minimized.

Copy link
Contributor

javifernandez commented Jan 22, 2018

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Apr 17, 2018

[css-align] Simple syntax for the Alignment shorthands
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

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Apr 19, 2018

[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

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Apr 19, 2018

[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

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Apr 20, 2018

[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 <jfernandez@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#552293}

aarongable pushed a commit to chromium/chromium that referenced this issue Apr 20, 2018

[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 <jfernandez@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#552293}

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Apr 20, 2018

[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 <jfernandez@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#552293}

hubot pushed a commit to WebKit/webkit that referenced this issue Apr 20, 2018

jfernandez@igalia.com
Update Alignment shorthands to the spec now that they are not ambiguous
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

Bug 1454587 [wpt PR 10502] - [css-align] Simple syntax for the Alignm…
…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 <jfernandez@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#552293}

--

wpt-commits: 4736f3fac17173f5e72c307ae586816169873fde
wpt-pr: 10502

mykmelez pushed a commit to mozilla/gecko that referenced this issue May 1, 2018

Bug 1454587 [wpt PR 10502] - [css-align] Simple syntax for the Alignm…
…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 <jfernandez@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#552293}

--

wpt-commits: 4736f3fac17173f5e72c307ae586816169873fde
wpt-pr: 10502
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.