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

alignment shorthand parsing is incorrect #16391

Closed
bzbarsky opened this issue Apr 12, 2017 · 5 comments
Closed

alignment shorthand parsing is incorrect #16391

bzbarsky opened this issue Apr 12, 2017 · 5 comments
Assignees

Comments

@bzbarsky
Copy link
Contributor

@bzbarsky bzbarsky commented Apr 12, 2017

When #16322 landed we fixed some Gecko failures, but picked up a bunch of new ones:

FAIL | layout/style/test/test_property_syntax_errors.html | invalid value 'center safe' not accepted for 'place-content' property
got "center safe", expected ""

FAIL | layout/style/test/test_property_syntax_errors.html | invalid value 'unsafe start' not accepted for 'place-content' property
got "start unsafe", expected ""

@tamamu @upsuper

@bzbarsky
Copy link
Contributor Author

@bzbarsky bzbarsky commented Apr 12, 2017

Also:

FAIL | layout/style/test/test_property_syntax_errors.html | invalid value 'end legacy left' not accepted for 'place-items' property
got "end left legacy", expected ""

@KWierso
Copy link
Contributor

@KWierso KWierso commented Apr 13, 2017

These are actually still failing on autoland (and since I merged, assuming they'd be fixed, mozilla-central). Any idea what I'd need to do to properly annotate these failures?

@KWierso
Copy link
Contributor

@KWierso KWierso commented Apr 13, 2017

The expectations update that landed earlier accidentally omitted the ".html" in the test name, causing it to not apply. Fixed in https://hg.mozilla.org/integration/autoland/rev/b3ef306a6bf0

@nox
Copy link
Member

@nox nox commented Apr 20, 2017

Why are these values forbidden? I can't find anything related to them in the spec.

@nox
Copy link
Member

@nox nox commented Apr 20, 2017

Oh I see, while it is a shorthand, it doesn't mean all the values from the longhands are allowed.

@nox nox self-assigned this Apr 20, 2017
nox added a commit to nox/servo that referenced this issue Apr 21, 2017
nox added a commit to nox/servo that referenced this issue Apr 21, 2017
nox added a commit to nox/servo that referenced this issue Apr 21, 2017
bors-servo added a commit that referenced this issue Apr 21, 2017
Properly parse alignment shorthands (fixes #16391)

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16558)
<!-- Reviewable:end -->
chenpighead added a commit to chenpighead/servo that referenced this issue Apr 24, 2017
avadacatavra added a commit to avadacatavra/servo that referenced this issue Apr 25, 2017
sadmansk added a commit to sadmansk/servo that referenced this issue May 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.