Twenty Twenty-One: Investigate duplicate rules in `ie-editor.css`

[SergeyBiryukov]

Spun off from ticket:53560#comment:19:

It seems weird that these rules are fully duplicated in the assets/css/ie-editor.css file:

select {
	border: 3px solid #39414d;
	border-radius: 0;
	color: #28303d;
	font-size: 1.125rem;
	-moz-appearance: none;
	-webkit-appearance: none;
	appearance: none;
	padding: 10px 30px 10px 10px;
	background: #fff url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>") no-repeat;
	background-position: right 10px top 60%;
}

select:focus {
	border: 3px solid #39414d;
	border-radius: 0;
	color: #28303d;
	font-size: 1.125rem;
	-moz-appearance: none;
	-webkit-appearance: none;
	appearance: none;
	padding: 10px 30px 10px 10px;
	background: #fff url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>") no-repeat;
	background-position: right 10px top 60%;
}

while they are properly combined in assets/css/style-editor.css:

select,
select:focus {
	border: var(--form--border-width) solid var(--form--border-color);
	border-radius: var(--form--border-radius);
	color: var(--form--color-text);
	font-size: var(--form--font-size);
	-moz-appearance: none;
	-webkit-appearance: none;
	appearance: none;
	padding: var(--form--spacing-unit) calc(3 * var(--form--spacing-unit)) var(--form--spacing-unit) var(--form--spacing-unit);
	background: var(--global--color-white) url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>") no-repeat;
	background-position: right var(--form--spacing-unit) top 60%;
}

Looking closer, ie-editor.css has a lot more unnecessarily duplicated rules, for example:

.has-background .has-link-color a {
	color: #28303d;
}

.has-background.has-link-color a {
	color: #28303d;
}

I would expect to see this instead:

.has-background .has-link-color a,
.has-background.has-link-color a {
	color: #28303d;
}

Could it be an issue with PostCSS configuration? I've noticed that postcss.config.js for the theme requires the postcss-discard-duplicates module, but it seems like it's not being used, or I'm misunderstanding how it works.

[SergeyBiryukov]

Thanks for moving this to a new ticket! Just changing the Reporter field for clarity, since it was my comment :)

[ryelle]

The postcss-discard-duplicates plugin gets rid of duplicate rules, it wouldn't combine different selectors like that.

It looks like it's probably the postcss-css-variables plugin, which is used to support IE11 and below. In fact, there's an issue for ​unnecessarily splitting up comma-separated rules, which seems to be what's happening here. The CSS in style-editor.css

.has-background .has-link-color a,
.has-background.has-link-color a {
	color: var(--wp--style--color--link, var(--global--color-primary));
}

is transformed to this in ie-editor.css

.has-background .has-link-color a {
	color: #28303d;
}

.has-background.has-link-color a {
	color: #28303d;
}

They suggest using a minification plugin or ​postcss-merge-rules to merge these back together. … Since I'm here, I gave the merge-rules plugin a try and it works surprisingly well. I don't have my IE machine up at the moment, so I can't test the output, but it looks correct.

Just noting, this isn't the first issue we've had with postcss-css-variables - see #52158 where the postcss-discard-duplicates plugin was added to clean up another bug.

[Shaharyar10]

Looking closer, assets/css/ie-editor.css has a lot more unnecessarily duplicated rules,

blockquote cite {
	font-weight: normal;
	letter-spacing: normal;
}

blockquote footer {
	font-weight: normal;
	letter-spacing: normal;
}

I would expect to see this instead:

blockquote cite,
blockquote footer {
	font-weight: normal;
	letter-spacing: normal;
}

also for this

blockquote.alignleft p {
	font-size: 1.125rem;
	max-width: inherit;
	width: inherit;
}

blockquote.alignright p {
	font-size: 1.125rem;
	max-width: inherit;
	width: inherit;
}

which should be like

blockquote.alignleft p,
blockquote.alignright p {
	font-size: 1.125rem;
	max-width: inherit;
	width: inherit;
}

Also there is another one

blockquote.alignleft cite {
	font-size: 1rem;
	letter-spacing: normal;
}

blockquote.alignleft footer {
	font-size: 1rem;
	letter-spacing: normal;
}

blockquote.alignright cite {
	font-size: 1rem;
	letter-spacing: normal;
}

blockquote.alignright footer {
	font-size: 1rem;
	letter-spacing: normal;
}

and it should be like

blockquote.alignleft cite,
blockquote.alignleft footer,
blockquote.alignright cite,
blockquote.alignright footer {
	font-size: 1rem;
	letter-spacing: normal;
}

[Shaharyar10]

Created patch.

[Shital Patel]

I added css in ie-editor.css and ie.css . please check and let me know if any changes.

[mukesh27]

We already fix a similar issue before so I think we have to go ahead with the solution suggested below.

Replying to ryelle:

They suggest using a minification plugin or ​postcss-merge-rules to merge these back together. … Since I'm here, I gave the merge-rules plugin a try and it works surprisingly well. I don't have my IE machine up at the moment, so I can't test the output, but it looks correct.

Just noting, this isn't the first issue we've had with postcss-css-variables - see #52158 where the postcss-discard-duplicates plugin was added to clean up another bug.

@shital-patel @Shaharyar10 Thanks for the patch. IE related CSS is generated automatically in the build process.

[poena]

I tried two things:
-The pull request as is
-Adding only the following changes to the theme (trunk):

Package.json, ​added {{{"postcss-merge-rules": "^5.0.2"}}}, to devDependencies.
(Deleted the lock file to regenerate it)

postcss.config.js ​added {{{require('postcss-merge-rules')}}} to plugins.

I saw no issues on npm install.
When I try to generate the updated CSS files, npm run start fails with:

assets/css/ie-editor.css
 1257:43  ✖  Unexpected empty block   block-no-empty

assets/css/ie.css
 2722:43  ✖  Unexpected empty block   block-no-empty
 4205:43  ✖  Unexpected empty block   block-no-empty

-There are no empty blocks on these lines.

If I only run the build:ie or build:ie-editor commands,
there are no error reports or warnings and most of the duplicate CSS is solved.

I am not sure how to solve the above problem with npm run start.

If it matters, my version of NPM is 6.14.15 (because Gutenberg is not compatible with higher and I use the same environment for testing both)

[poena]

As for the resulting CSS, I do not see any new issue when I test the theme in IE11.

[ryelle]

I just rebased ​the branch to get it up to date, and I'm not seeing any build errors. Since the built CSS looks good, I think this should get committed.

[peterwilsoncc]

I am seeing linting errors running npm run lint:scss but they seem unrelated to this change as they exist in trunk without the patch applied.

I agree this can be committed as the linting errors are pre-existing.

[peterwilsoncc]

In 52238:

Twenty Twenty-One: Combine duplicate CSS rules.

Use the postcss-merge-rules package when building TT1 stylesheets to reduce duplicate code in production CSS.

Props SergeyBiryukov, ryelle, Shaharyar10, shital-patel, mukesh27, poena.
Fixes #53605.

committed in r52238.