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

text-decoration prefixing inconsistent #1473

Closed
h3rmanj opened this issue Sep 7, 2022 · 8 comments · Fixed by browserslist/caniuse-lite#104
Closed

text-decoration prefixing inconsistent #1473

h3rmanj opened this issue Sep 7, 2022 · 8 comments · Fixed by browserslist/caniuse-lite#104

Comments

@h3rmanj
Copy link

h3rmanj commented Sep 7, 2022

text-decoration support was added in #857. However, it doesn't prefix for basic values. This can cause inconsistent situations;

span {
  text-decoration: underline dashed;
}

span.underline-only {
  text-decoration: underline;
}

/* after autoprefix */
span {
  -webkit-text-decoration: underline dashed;
  text-decoration: underline dashed;
}

span.underline-only {
  text-decoration: underline;
}
<span class="underline-only">Underlined text</span>

The span would then have a normal solid underline in most browsers, but in safari it will prefer the prefixed one, even though it should be overridden.

For my situation, I'm using a third party design system, and don't manage the bundling/processing myself (create-react-app), which makes it hard to debug and fix myself.

The solution would be to always apply the prefix on text-decoration, or have some global property that prefixes all if it has been prefixed once.

h3rmanj added a commit to h3rmanj/autoprefixer that referenced this issue Sep 7, 2022
@ai
Copy link
Member

ai commented Sep 7, 2022

I do not like disable the feature since it will create breaking changes for more users.

What do you think about adding the warning?

@h3rmanj
Copy link
Author

h3rmanj commented Sep 7, 2022

I'm not sure I understand. I'm not suggesting removing the feature, but adding prefix to text-decoration with basic values as well. I wouldn't think that was any more breaking than adding the feature in the first place.

Given the output and result in my example, would it not be considered a bug? Is there a specific situation I'm not thinking about that would break?

@ai
Copy link
Member

ai commented Sep 7, 2022

Got it!

I am little worry of adding invalid prefixed value. But I agree that we need to fix your case somewhow.

@ai
Copy link
Member

ai commented Sep 7, 2022

The problem is that we already do not prefix a few properties if value is not supported.

check() is also used in image-rendering, background-clip, animation (and a few grid properties).

It will be strange to fix only one hack.

@h3rmanj
Copy link
Author

h3rmanj commented Sep 8, 2022

Wait, so -webkit-text-decoration only supports multiple values?
If that is the case, in theory you should never have a stylesheet with both text-decoration basic values and multiple values, as it's a high chance it will bug out on safari 🙃

@xec
Copy link

xec commented Sep 8, 2022

As far as I can tell, single value -webkit-text-decoration is supported in safari (only tested on iphone)

div {
  /* will apply underline in safari */
  -webkit-text-decoration: underline;
}

https://jsfiddle.net/78z36yam/1/

So if autoprefixer would generate css like this I think it would work fine in all cases

span {
  text-decoration: underline dashed;
}
span.underline-only {
  text-decoration: underline;
}

/* after autoprefix */
span {
  -webkit-text-decoration: underline dashed;
  text-decoration: underline dashed;
}
span.underline-only {
  -webkit-text-decoration: underline;
  text-decoration: underline;
}

Although the unprefixed text-decoration: underline; is also supported by safari, it does not override the prefixed version from the span selector.

@romainmenke
Copy link
Contributor

romainmenke commented Sep 12, 2022

@ai We have a plugin specifically for shorthand text-decoration fallbacks.
https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-text-decoration-shorthand#readme

We can also fix this issue there to avoid adding a hack in autoprefixer.
Not everyone using autoprefixer also uses postcss-preset-env so it is also an imperfect solution :)


The root cause is that text-decoration unprefixed in Safari is still a non-shorthand property.

It does not override omitted constituent properties with their initial values as a shorthand property would.

It is equivalent to text-decoration-line.

span.underline-only {
  text-decoration: underline;
}

/* in Safari same as : */

span.underline-only {
  text-decoration-line: underline;
}

-webkit-text-decoration however is a shorthand property in Safari and does set omitted constituent properties to their initial values.

see : https://developer.mozilla.org/en-US/docs/Web/CSS/text-decoration#formal_definition

span.underline-only {
  -webkit-text-decoration: underline;
  text-decoration: underline;
}

/* in Safari this behaves like : */

span.underline-only {
  text-decoration: underline currentColor solid;
  text-decoration-line: underline;
}

As far as I know it should be fine to always add -webkit-text-decoration for Safari.

@romainmenke
Copy link
Contributor

romainmenke commented Sep 12, 2022

Accidentally closed by linked PR, should be re-opened

I used "resolve" in a sentence followed by a link to this issue, GitHub isn't able to grasp natural language (yet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants