postcss / autoprefixer

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

[Firefox] -moz-read-only is not duplicated with all associated css selectors #1301

Open albanlorillard opened 4 years ago

albanlorillard commented 4 years ago

Hello,

Description

This issue is refered to https://github.com/angular/angular-cli/issues/17325 Since I upgrade Angular 8 to 9, autoprefixer has updated from 9.6.1 to 9.7.1. Due to this update, behaviour of autoprefixer has changed about a piece of my code concerned Read-Only css rule.

This issue appear when read-only is in a same block of other css selector associate to the same css code. For example :

.element1:focus,
.element1:read-only {
  background-color: red;
}

Now, in 9.7.1, render :

/* prefixed by https://autoprefixer.github.io (PostCSS: v7.0.26, autoprefixer: v9.7.3) */
.element1:-moz-read-only {
  background-color: red;
}
.element1:focus,
.element1:read-only {
  background-color: red;
}

But should render, like previously :

.element1:-moz-read-only,
.element1:focus {
 background-color:red
}
.element1:focus,
.element1:read-only {
 background-color:red
}

As you can seen, .element1:focus, is never associate to a code block with .element1:-moz-read-only.

Because :read-only is unknown for firefox, all the code block that include .element1:read-only is never interpreted included other rules of this same code block. ( On the example, :focus is never interpreted, because on firefox, the code blocks failed due to unkown attribute :read-only ) On previous version that work because other rules (here :focus) is duplicated in the generated code block that include :-moz-read-only.

Minimal reproduction :

I previously create 2 scratchs projects for angular, where we can see the bug Here is a repository with angular 9, (so, with autoprefixer 9.7.x) where the bug appear : https://github.com/albanlorillard/angular9-readonly-bug

Here is a repository with angular 8, (so, with autoprefixer 9.6.x) where the bug NOT appear : https://github.com/albanlorillard/angular8-readonly-bug

Browsers tested :

Firefox developer 75.0b9 (64 bits) Firefox 73.0 (64-bit)

ai commented 4 years ago

As I understand, having .element1:-moz-read-only, .element1:focus together is a wrong output, because Firefox will ignore the rule because of unknown :focus selector.

Can you explain better why the wrong code from previous version workes for you and new versions doesn’t work?

albanlorillard commented 4 years ago

Thanks for your quick answer and sorry, I will tried to clarify.

:focus was just an example it could be any random others rules.

When I pass autoprefixer on this block code :

randomSelector1,
.element1:read-only,
randomSelector2,
randomSelector3 {
 /* css rules */
}

Now, autoprefixer not repeat my selectors on the rules which :-moz-read-only is generated :

/* Code interpreted on firefox : */ 
.element1:-moz-read-only {
/* css rules */
}

/* Code not interpreted on firefox due to :read-only, because it's an unknown pseudo-class for firefox, that result to an error*/ 
randomSelector1,
.element1:read-only,
randomSelector2,
randomSelector3 {
 /* css rules */
}

In consequence, on last version of autoprefixer, randomSelector1, randomSelector2, randomSelector3, is not associate to any css rules for firefox browser. But it does for any other browser like google chrome. (Because this last browser can interpreted this last piece of code)

Previously, on 3.6.x this selectors was correctly copied around the selector that have :moz-read-only pseudo-class

/* Code interpreted on firefox : */ 
randomSelector1,
.element1:-moz-read-only,
randomSelector2,
randomSelector3
 {
 /* css rules */
}

/* Code not interpreted on firefox, but doesn't matter, equivalent work fine on the top */ 
randomSelector1,
.element1:read-only,
randomSelector2,
randomSelector3 {
 /* css rules */
}

(Related issue on Bugzilla about :read-only pseudo-class : https://bugzilla.mozilla.org/show_bug.cgi?id=312971 / https://bugzilla.mozilla.org/show_bug.cgi?id=313111 )

ai commented 4 years ago

Why do you need all these selectors together?

randomSelector1,
.element1:-moz-read-only,
randomSelector2,
randomSelector3
albanlorillard commented 4 years ago

Because on my original CSS code, I gathered many selectors in a same piece of css code

If you prefer,this piece of code :

randomSelector1,
.element1:read-only,
randomSelector2,
randomSelector3 {
 /* css rules */
}

Not work at all on Firefox. Firefox not just ignore .element1:read-only and execute code for other selectors. It ignore css rules for all selectors, because for him, their is a syntax error (I imagine) due to unknown :read-only pseudoclass

ai commented 4 years ago

Got it.

I do not have time for this issue in the next few weeks.

You can help me by sending PR to this file: https://github.com/postcss/autoprefixer/blob/master/lib/selector.js

rpearce commented 4 years ago

@albanlorillard Were you able to find a way to work around this for now? I have the same issue.

rpearce commented 4 years ago

Can confirm that it works in 9.6.4 but breaks in 9.6.5

albanlorillard commented 4 years ago

Hi !

I've not starting at all because I've not found the time for the moment. If you want to try to solve no problem for me ;)

Else, I will try to found some time next week

rpearce commented 4 years ago

I will look at it, but if you don't see anything from me in a week, assume that it might not happen

shrpne commented 4 years ago

Am I right, that solution to the issue is to revert this commit: f6264415c401edfe60acef9c0bbab86b206b6234 ?

ai commented 4 years ago

@shrpne that commit fixed abouther bug. BY reverting it we will close one issue by openning the another.

ai commented 4 years ago

/cc @fanich37

shrpne commented 4 years ago

Is not this #1196 issue was invalid? Because what happens now with -moz-read-only is exactly what that issue was proposing.

ai commented 4 years ago

@shrpne nope. The origin issue was about not putting all prefixes to the selector (if Safari will find -moz- in the selector it will ignore the whole rule).

Seems like we had an issue selector cleaning implementation and we need to fix it

fanich37 commented 4 years ago

The origin issue was about not putting all prefixes to the selector

@ai May be it wasn't a good idea to separate them. Futhermore it out of autoprefixer scope to optimize the code somehow.

I haven't look through the code carefully but at first glance it's this line that causes such an issue" https://github.com/postcss/autoprefixer/blob/0283e319ff7bcde0454c959036e0d91ae0bb326f/lib/selector.js#L68

ai commented 4 years ago

Oops, I missed the Expected and Actual in the original issue.

Yeap, let’s revert it.

ai commented 4 years ago

@fanich37 can I ask you to send PR and add tests to prevent this regression in the future?

fanich37 commented 4 years ago

Oops, I missed the Expected and Actual in the original issue.

Yeap, let’s revert it.

Reverting will open another bug as you mentioned above.

fanich37 commented 4 years ago

@fanich37 can I ask you to send PR and add tests to prevent this regression in the future?

I'll try to check it ASAP. Just removing this line: https://github.com/postcss/autoprefixer/blob/0283e319ff7bcde0454c959036e0d91ae0bb326f/lib/selector.js#L68 won't help since we get this result:

.el1:focus, .el2::-moz-selection, .el3:read-only {
  background-color: red;
}

.el1:focus, .el2::selection, .el3:-moz-read-only {
  background-color: red;
}

.el1:focus,
.el2::selection,
.el3:read-only {
  background-color: red;
}

And the first rule generated with autoprefixer is still broken for Firefox. And it's also broken in 9.6.4:

.el1:focus,
.el2::-moz-selection,
.el3:read-only {
  background-color: red;
}

.el1:focus,
.el2::selection,
.el3:read-only {
  background-color: red;
}
JustBeYou commented 3 years ago

Using multiple pseudoclasses on the same element is also broken.

.example:any-link:read-only {}

Output:

/*
* Prefixed by https://autoprefixer.github.io
* PostCSS: v7.0.29,
* Autoprefixer: v9.7.6
* Browsers: >0%
*/

.example:any-link:-moz-read-only {}
.example:-webkit-any-link:read-only {}
.example:-moz-any-link:read-only {}
.example:any-link:read-only {}

Expected:

.example:any-link:read-only {}
.example:-moz-any-link:-moz-read-only {}
.example:any-link:-webkit-read-only {}