autoprefixer icon indicating copy to clipboard operation
autoprefixer copied to clipboard

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

Open albanlorillard opened this issue 5 years ago • 20 comments
trafficstars

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)

albanlorillard avatar Apr 14 '20 08:04 albanlorillard

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?

ai avatar Apr 14 '20 12:04 ai

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 )

albanlorillard avatar Apr 14 '20 21:04 albanlorillard

Why do you need all these selectors together?

randomSelector1,
.element1:-moz-read-only,
randomSelector2,
randomSelector3

ai avatar Apr 14 '20 21:04 ai

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

albanlorillard avatar Apr 14 '20 21:04 albanlorillard

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

ai avatar Apr 14 '20 21:04 ai

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

rpearce avatar Apr 29 '20 16:04 rpearce

Can confirm that it works in 9.6.4 but breaks in 9.6.5

rpearce avatar Apr 29 '20 16:04 rpearce

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

albanlorillard avatar Apr 29 '20 16:04 albanlorillard

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

rpearce avatar Apr 29 '20 16:04 rpearce

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

shrpne avatar May 29 '20 16:05 shrpne

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

ai avatar May 29 '20 17:05 ai

/cc @fanich37

ai avatar May 29 '20 17:05 ai

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

shrpne avatar May 29 '20 17:05 shrpne

@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

ai avatar May 29 '20 17:05 ai

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

fanich37 avatar May 30 '20 20:05 fanich37

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

Yeap, let’s revert it.

ai avatar May 30 '20 20:05 ai

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

ai avatar May 30 '20 20:05 ai

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 avatar May 30 '20 20:05 fanich37

@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;
}

fanich37 avatar Jun 01 '20 12:06 fanich37

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 {}

JustBeYou avatar May 21 '21 13:05 JustBeYou