WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 2 months ago

#53605 assigned defect (bug)

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

Reported by: SergeyBiryukov Owned by:
Milestone: 5.9 Priority: normal
Severity: normal Version: trunk
Component: Bundled Theme Keywords: needs-testing has-patch good-first-bug
Focuses: Cc:

Description

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.

Attachments (2)

53605.diff (1.2 KB) - added by Shaharyar10 3 months ago.
Created patch.
53605.1.diff (2.5 KB) - added by Shital Patel 3 months ago.
I added css in ie-editor.css and ie.css . please check and let me know if any changes.

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
4 months ago

  • Reporter changed from desrosj to SergeyBiryukov

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

#2 follow-up: @ryelle
4 months ago

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.

#3 @SergeyBiryukov
4 months ago

  • Milestone changed from Future Release to 5.9

#4 @Shaharyar10
3 months ago

  • Version set to trunk

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
3 months ago

Created patch.

@Shital Patel
3 months ago

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

#5 @Shital Patel
3 months ago

  • Keywords has-patch added; good-first-bug removed

#6 @Shital Patel
3 months ago

  • Owner set to Shital Patel
  • Status changed from new to accepted

#7 @Shital Patel
3 months ago

  • Keywords good-first-bug added

#8 in reply to: ↑ 2 @mukesh27
2 months ago

  • Owner Shital Patel deleted
  • Status changed from accepted to assigned

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.

Note: See TracTickets for help on using tickets.