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-text-decor] Add parsing tests for text-decoration-line #16347

Merged
merged 1 commit into from Apr 15, 2019

Conversation

mrego
Copy link
Member

@mrego mrego commented Apr 15, 2019

No description provided.

@mrego
Copy link
Member Author

mrego commented Apr 15, 2019

This is a parsing text for text-decoration-line based on the ones done by @ericwilligers in other specs.

My only doubt is regarding things like text-decoration-line: overline underline, what should be the specified value in that case?

The property definition says:

Canonical order: per grammar

And I found this text in CSSOM spec:

If certain component values can appear in any order without changing the meaning of the value (a pattern typically represented by a double bar || in the value syntax), reorder the component values to use the canonical order of component values as given in the property definition table.

So I understand that the right answer is underline overline.

That's what these tests are checking, and the ones with different order fail in Chromium/WebKit while they pass in Firefox. If actually the order doesn't matter and it could be either overline underline or underline overline then I'd need to update the tests accordingly.

Could someone clarify this topic? Thanks!

CC @frivoal @emilio

@emilio
Copy link
Contributor

emilio commented Apr 15, 2019

That's what these tests are checking, and the ones with different order fail in Chromium/WebKit while they pass in Firefox. If actually the order doesn't matter and it could be either overline underline or underline overline then I'd need to update the tests accordingly.

The order is supposed to matter. Firefox is right here.

Copy link
Contributor

@ewilligers ewilligers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix Blink.

@mrego
Copy link
Member Author

mrego commented Apr 15, 2019

I'll fix Blink.

I've a patch already. :)

@mrego mrego merged commit 8b6ea4b into web-platform-tests:master Apr 15, 2019
@mrego mrego deleted the text-decoration-line branch April 15, 2019 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants