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.

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

[css-text-decor] Can text-decoration-thickness mix percentages and lengths? #9469

Open
nt1m opened this issue Oct 15, 2023 · 6 comments · May be fixed by #10699
Open

[css-text-decor] Can text-decoration-thickness mix percentages and lengths? #9469

nt1m opened this issue Oct 15, 2023 · 6 comments · May be fixed by #10699
Labels
Closed Accepted as Obvious Bugfix css-text-decor-4

Comments

@nt1m
Copy link
Member

nt1m commented Oct 15, 2023

There's a WPT testing that text-decoration-thickness takes calc() mixing percentages and lengths.
https://github.com/web-platform-tests/wpt/blob/b5c73fa21b/css/css-text-decor/text-decoration-thickness-valid.html#L24-L25

Except the spec specifies the syntax to be: auto | from-font | <length> | <percentage>, which means that they can't actually be mixed? If they are allowed to be mixed, the type should probably be <length-percentage>.

It seems like the mixing is supported in Firefox / Chrome. Safari doesn't currently support percentages.

cc @kojiishi @fantasai @shivamidow
@nt1m
Copy link
Member Author

nt1m commented Oct 15, 2023

cc @drott @jfkthame

@Loirooriol
Copy link
Contributor

I would expect <length-percentage>.

@shivamidow shivamidow mentioned this issue Oct 16, 2023
Merged
@shivamidow
Copy link
Member

Yeah, I also think <length-percentage> makes sense.

@nt1m nt1m added the Agenda+ label Oct 16, 2023
@drott
Copy link
Collaborator

drott commented Oct 16, 2023

@nt1m wrote:

If they are allowed to be mixed, the type should probably be .

Agree, that would make sense. We parse as ConsumeLengthOrPercent, (impl)

fantasai added a commit that referenced this issue Oct 20, 2023
@fantasai
[css-text-decor-4] Clarify that <length> and <percentage> can be comb…
27580df 
…ined in text-decoration-thickness and text-underline-offset #9469
@fantasai
Copy link
Collaborator

Quite sure this was not intentional. Likely at the time this was written, there was no distinction between <length-percentage> and <length> | <percentage>. Fixed.

@Loirooriol
Copy link
Contributor

@fantasai You fixed the grammar, but the prose below still tries to define <length> and <percentage> separately, instead of <length-percentage>.

@Loirooriol Loirooriol reopened this Nov 20, 2023
nt1m added a commit that referenced this issue Aug 6, 2024
@nt1m
[css-text-decor-4] Define <length-percentage> in one definition
ec444e7 
Instead of having a separate definition for `<length>` and `<percentage>`.

Fixes #9469
@nt1m nt1m linked a pull request Aug 6, 2024 that will close this issue
Open
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 as Obvious Bugfix css-text-decor-4
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[css-text-decor-4] Define <length-percentage> as a single type w3c/csswg-drafts
5 participants
@fantasai @drott @nt1m @shivamidow @Loirooriol