postcss / autoprefixer

Parse CSS and add vendor prefixes to rules by Can I Use
https://twitter.com/autoprefixer
MIT License
21.57k stars 1.25k forks source link

`text-decoration` prefixing inconsistent #1473

Closed h3rmanj closed 1 year ago

h3rmanj commented 1 year ago

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.

ai commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Got it!

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

ai commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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)