Make WordPress Core

Opened 6 weeks ago

Closed 5 weeks ago

Last modified 5 weeks ago

#65993 closed defect (bug) (fixed)

Running `grunt precommit:css` in old branches results in uncommitted changes

Reported by: desrosj Owned by: lancewillett
Priority: normal Milestone: 7.2
Component: Build/Test Tools Version:
Severity: normal Keywords: has-patch
Cc: Focuses:

Description

In r63315, a grunt precommit:css step was added to the reusable workflow responsible for testing Core's build process.

This has exposed the fact that the precommit:css command was not run after the browser usage database was updated last in the 5.2 through 5.7 branches.

For example, see ​this workflow log for the 5.2 branch.

These changes are very minor. They should either be committed, or a new input should be added to the reusable workflow that accepts a boolean to enable the precommit:css step.

Change History (35)

This ticket was mentioned in ​PR #13318 on ​WordPress/wordpress-develop by ​@khokansardar.


6 weeks ago
#1

  • Keywords has-patch added

The Test Build Processes workflow now fails for the 5.2 through 5.7 branches.

The precommit:css step added in [63315] runs Autoprefixer against whichever branch is checked out. Those branches have not had the task run since the browser usage database was last updated, so it rewrites tracked CSS files and the git diff --exit-code step that follows the build then fails. Since those branches call this reusable workflow at @trunk, this adds a test-css input mirroring the existing test-emoji one, letting each branch opt out.

It defaults to true, so no current caller changes behaviour; the affected branches still need test-css: false added alongside their existing test-emoji: false.

Trac ticket: https://core.trac.wordpress.org/ticket/65993

## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Tracing the CI failure to the precommit:css step and drafting the workflow input. All changes were reviewed and validated by me.

This ticket was mentioned in ​Slack in #core by desrosj. ​View the logs.


5 weeks ago

​@lancewillett commented on ​PR #13318:


5 weeks ago
#3

@itzmekhokan, after this lands on trunk, please open one follow-up PR each against the affected branches:

  • [ ] 5.2—PR:
  • [ ] 5.3—PR:
  • [ ] 5.4—PR:
  • [ ] 5.5—PR:
  • [ ] 5.6—PR:
  • [ ] 5.7—PR:

In each PR, add test-css: false beside the existing test-emoji: false in both with: blocks in .github/workflows/test-build-processes.yml. Please link Trac #65993 and confirm all Test Build Processes jobs pass.

Once all six are green, please comment here with the PR links and results. We’ll then coordinate the required second-core-committer reviews and SVN branch commits.

#4 @lancewillett
5 weeks ago

In 63445:

Build/Test Tools: Allow the CSS precommit task to be skipped.

The reusable build-process workflow runs precommit:css against the checked-out branch. On branches 5.2 through 5.7, the task rewrites tracked CSS and causes the clean-tree check to fail.

Add a test-css input so affected branch callers can skip the task. It defaults to true, preserving behavior for existing callers.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13318

Props desrosj, khokansardar.
See #65993.

#5 @lancewillett
5 weeks ago

  • Owner set to lancewillett
  • Status new → accepted

Let's keep this ticket open until the backport changes land; see ​https://github.com/WordPress/wordpress-develop/pull/13318#issuecomment-5527245754

​@lancewillett commented on ​PR #13318:


5 weeks ago
#7

Slack failures indicate we still need the backport PRs to land: ​https://wordpress.slack.com/archives/C02RQBWTW/p1788453035296919

​@desrosj commented on ​PR #13318:


5 weeks ago
#8

I'm a pretty torn on this one.

The fact that these CSS changes have not been committed to the branch is a bug. It's also a sign that this new step in the workflow is doing it's job and flagging legitimate problems, even ones minor in nature.

While precommit:css usually results in minor changes that only affect users who have really old versions of browsers installed (which, in most cases, we have not supported for some time), it is possible that this could flag something more problematic in the future.

I am currently of the opinion that we should just commit the changes that were missed to these 5 or so branches instead.

​@desrosj commented on ​PR #13318:


5 weeks ago
#9

To make sure I was not recommending that we make changes that were too substantive, I analyzed the actual changes:

## 5.7 branch

28 KB src/wp-admin/css/about.css

## 5.6 branch

20 KB src/wp-admin/css/admin-menu.css
72 KB src/wp-admin/css/common.css
68 KB src/wp-admin/css/customize-controls.css
24 KB src/wp-admin/css/customize-nav-menus.css
16 KB src/wp-admin/css/customize-widgets.css
8 KB src/wp-admin/css/site-health.css
44 KB src/wp-admin/css/themes.css
20 KB src/wp-admin/css/widgets.css
24 KB src/wp-includes/css/admin-bar.css
36 KB src/wp-includes/css/editor.css
56 KB src/wp-includes/css/media-views.css

## 5.5 branch

20 KB src/wp-admin/css/admin-menu.css
72 KB src/wp-admin/css/common.css
68 KB src/wp-admin/css/customize-controls.css
24 KB src/wp-admin/css/customize-nav-menus.css
16 KB src/wp-admin/css/customize-widgets.css
8 KB src/wp-admin/css/site-health.css
44 KB src/wp-admin/css/themes.css
20 KB src/wp-admin/css/widgets.css
24 KB src/wp-includes/css/admin-bar.css
36 KB src/wp-includes/css/editor.css
56 KB src/wp-includes/css/media-views.css

## 5.4 branch

20 KB src/wp-admin/css/admin-menu.css
72 KB src/wp-admin/css/common.css
68 KB src/wp-admin/css/customize-controls.css
24 KB src/wp-admin/css/customize-nav-menus.css
16 KB src/wp-admin/css/customize-widgets.css
40 KB src/wp-admin/css/themes.css
20 KB src/wp-admin/css/widgets.css
28 KB src/wp-includes/css/admin-bar.css
36 KB src/wp-includes/css/editor.css
56 KB src/wp-includes/css/media-views.css

## 5.3 branch

20 KB src/wp-admin/css/admin-menu.css
72 KB src/wp-admin/css/common.css
68 KB src/wp-admin/css/customize-controls.css
24 KB src/wp-admin/css/customize-nav-menus.css
16 KB src/wp-admin/css/customize-widgets.css
40 KB src/wp-admin/css/themes.css
20 KB src/wp-admin/css/widgets.css
28 KB src/wp-includes/css/admin-bar.css
36 KB src/wp-includes/css/editor.css
56 KB src/wp-includes/css/media-views.css

## 5.2 branch

20 KB src/wp-admin/css/admin-menu.css
72 KB src/wp-admin/css/common.css
68 KB src/wp-admin/css/customize-controls.css
24 KB src/wp-admin/css/customize-nav-menus.css
16 KB src/wp-admin/css/customize-widgets.css
40 KB src/wp-admin/css/themes.css
20 KB src/wp-admin/css/widgets.css
36 KB src/wp-includes/css/editor.css
52 KB src/wp-includes/css/media-views.css

As for the changes themselves, nearly all of the changed lines are removing -moz-user-select: none; rules. Firefox has not required this vendor prefix since version 68 (they're currently at 150+). There are a small number of min-width: -moz-fit-content additions in a few of the branches.

In summary, the files modified combine for less than 400KB in size that would be added to the next auto-update for each, sould one be required.

​@lancewillett commented on ​PR #13318:


5 weeks ago
#10

I'm in favor of fixing the CSS instead of tweaking the test behavior. Great feedback @desrosj

@ itzmekhokan What do you think about reverting this in trunk, not building the branches, and fixing the CSS rules as suggested?

​@lancewillett commented on ​PR #13318:


5 weeks ago
#11

After more thought, I do agree we should flip this to fix CSS files instead.

Suggested direction (open to more feedback!):

  1. Revert r63445 on trunk
  2. Replace the planned test-css: false backports with one PR per affected branch
  3. Each PR should contain only the output of npm run grunt precommit:css—no workflow changes
  4. Keep the CSS check enabled and confirm the build-process jobs pass on all three operating systems
  5. Close Trac #65993 after all six branch commits land

This fixes the code flagged by the check and preserves CI’s ability to detect future CSS drift.

@itzmekhokan, since you identified the failure and contributed the original fix, I’d like you to remain part of the solution. Would you be willing to prepare the six branch PRs with the generated CSS changes?

This ticket was mentioned in ​PR #13400 on ​WordPress/wordpress-develop by ​@khokansardar.


5 weeks ago
#12

The Test Build Processes workflow fails on the 5.7 branch. The precommit:css step added in [63315] runs Autoprefixer over src and rewrites tracked CSS, so the clean-tree check that follows it fails.

The task has not been run since the browser usage database pinned in package-lock.json was last updated. This commits its output and nothing else; every changed line is an Autoprefixer vendor prefix.

Following the discussion in PR 13318, this fixes the flagged CSS rather than skipping the check. The src jobs fail on Linux, Windows and macOS today, and all three produce an identical diff.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reproducing the CI failure locally and running npm run grunt precommit:css to regenerate the CSS. All changes were reviewed and validated by me.

This ticket was mentioned in ​PR #13401 on ​WordPress/wordpress-develop by ​@khokansardar.


5 weeks ago
#13

The Test Build Processes workflow fails on the 5.2 branch. The precommit:css step added in [63315] runs Autoprefixer over src and rewrites tracked CSS, so the clean-tree check that follows it fails.

The task has not been run since the browser usage database pinned in package-lock.json was last updated. This commits its output and nothing else; every changed line is an Autoprefixer vendor prefix.

Following the discussion in PR 13318, this fixes the flagged CSS rather than skipping the check. The src jobs fail on Linux, Windows and macOS today, and all three produce an identical diff.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reproducing the CI failure locally and running npm run grunt precommit:css to regenerate the CSS. All changes were reviewed and validated by me.

This ticket was mentioned in ​PR #13402 on ​WordPress/wordpress-develop by ​@khokansardar.


5 weeks ago
#14

The Test Build Processes workflow fails on the 5.3 branch. The precommit:css step added in [63315] runs Autoprefixer over src and rewrites tracked CSS, so the clean-tree check that follows it fails.

The task has not been run since the browser usage database pinned in package-lock.json was last updated. This commits its output and nothing else; every changed line is an Autoprefixer vendor prefix.

Following the discussion in PR 13318, this fixes the flagged CSS rather than skipping the check. The src jobs fail on Linux, Windows and macOS today, and all three produce an identical diff.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reproducing the CI failure locally and running npm run grunt precommit:css to regenerate the CSS. All changes were reviewed and validated by me.

This ticket was mentioned in ​PR #13403 on ​WordPress/wordpress-develop by ​@khokansardar.


5 weeks ago
#15

The Test Build Processes workflow fails on the 5.4 branch. The precommit:css step added in [63315] runs Autoprefixer over src and rewrites tracked CSS, so the clean-tree check that follows it fails.

The task has not been run since the browser usage database pinned in package-lock.json was last updated. This commits its output and nothing else; every changed line is an Autoprefixer vendor prefix.

Following the discussion in PR 13318, this fixes the flagged CSS rather than skipping the check. The src jobs fail on Linux, Windows and macOS today, and all three produce an identical diff.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reproducing the CI failure locally and running npm run grunt precommit:css to regenerate the CSS. All changes were reviewed and validated by me.

This ticket was mentioned in ​PR #13404 on ​WordPress/wordpress-develop by ​@khokansardar.


5 weeks ago
#16

The Test Build Processes workflow fails on the 5.5 branch. The precommit:css step added in [63315] runs Autoprefixer over src and rewrites tracked CSS, so the clean-tree check that follows it fails.

The task has not been run since the browser usage database pinned in package-lock.json was last updated. This commits its output and nothing else; every changed line is an Autoprefixer vendor prefix.

Following the discussion in PR 13318, this fixes the flagged CSS rather than skipping the check. The src jobs fail on Linux, Windows and macOS today, and all three produce an identical diff.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reproducing the CI failure locally and running npm run grunt precommit:css to regenerate the CSS. All changes were reviewed and validated by me.

This ticket was mentioned in ​PR #13405 on ​WordPress/wordpress-develop by ​@khokansardar.


5 weeks ago
#17

The Test Build Processes workflow fails on the 5.6 branch. The precommit:css step added in [63315] runs Autoprefixer over src and rewrites tracked CSS, so the clean-tree check that follows it fails.

The task has not been run since the browser usage database pinned in package-lock.json was last updated. This commits its output and nothing else; every changed line is an Autoprefixer vendor prefix.

Following the discussion in PR 13318, this fixes the flagged CSS rather than skipping the check. The src jobs fail on Linux, Windows and macOS today, and all three produce an identical diff.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Reproducing the CI failure locally and running npm run grunt precommit:css to regenerate the CSS. All changes were reviewed and validated by me.

​@khokansardar commented on ​PR #13318:


5 weeks ago
#18

@lancewillett @desrosj — agreed, fixing the CSS is the better outcome. Six PRs are open, one per branch, each containing only the output of npm run grunt precommit:css and no workflow changes:

All six are green. That is 36 of 36 Core running from src and Core running from build jobs passing on Linux, Windows and macOS. The build jobs were already passing before these changes; the src jobs were the failing ones.

Each branch was regenerated on Node 14 with npm ci, so Autoprefixer and caniuse-lite come from that branch's own package-lock.json. Before committing I diffed the result against the failing Core running from src logs for all six branches on all three operating systems, and every one of those 18 jobs produced a diff byte-identical to what is committed here.

The content matches your analysis: every deleted line is a -moz-user-select declaration, 5.3 through 5.6 each add one min-width: -moz-fit-content in admin-bar.css, and 5.7 adds -webkit-user-select and -ms-user-select to one rule in about.css.

That leaves the trunk revert of [63445], which needs a committer. Once that and the six branch commits land, https://core.trac.wordpress.org/ticket/65993 can be closed.

​@lancewillett commented on ​PR #13318:


5 weeks ago
#19

Looking good @itzmekhokan

@desrosj If you could be a 2nd reviewer on all these PRs I'll do the core commits

#20 @lancewillett
5 weeks ago

In 63465:

Build/Test Tools: Revert r63445.

The affected branches will regenerate and commit their CSS output instead of skipping the CSS precommit task. This preserves the check for future CSS drift.

Reverts r63445.

Props desrosj, khokansardar.
See #65993.

​@lancewillett commented on ​PR #13318:


5 weeks ago
#21

r63465 reverted r63445

The replacement CSS fixes are tracked in #13400, #13401, #13402, #13403, #13404, and #13405

Waiting on a 2nd core committer to review

​@desrosj commented on ​PR #13402:


5 weeks ago
#22

One note, I did have a line changed in the package-lock.json file locally flipping an https link to http.

I'm getting a lot warnings and notices running npm install locally (likely because of the old version of Node.js not liking my newer Macbook). Since CI is looking good without a change to the lock file, going to ignore those.

​@desrosj commented on ​PR #13318:


5 weeks ago
#23

Going to close this out as fixed because the original change was reverted. Let's use the individual PRs to follow up.

#24 @lancewillett
5 weeks ago

In 63468:

Build/Test Tools: Regenerate CSS for the 5.7 branch.

Regenerate CSS vendor prefixes to match the branch's browser support data. This keeps the CSS precommit task and clean-tree check passing.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13400

Reviewed by: desrosj.
Props khokansardar.
See #65993.

#26 @lancewillett
5 weeks ago

In 63469:

Build/Test Tools: Regenerate CSS for the 5.6 branch.

Regenerate CSS vendor prefixes to match the branch's browser support data. This keeps the CSS precommit task and clean-tree check passing.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13405

Reviewed by: desrosj.
Props khokansardar.
See #65993.

#28 @lancewillett
5 weeks ago

In 63470:

Build/Test Tools: Regenerate CSS for the 5.5 branch.

Regenerate CSS vendor prefixes to match the branch's browser support data. This keeps the CSS precommit task and clean-tree check passing.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13404

Reviewed by: desrosj.
Props khokansardar.
See #65993.

#30 @lancewillett
5 weeks ago

In 63471:

Build/Test Tools: Regenerate CSS for the 5.4 branch.

Regenerate CSS vendor prefixes to match the branch's browser support data. This keeps the CSS precommit task and clean-tree check passing.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13403

Reviewed by: desrosj.
Props khokansardar.
See #65993.

#32 @lancewillett
5 weeks ago

In 63472:

Build/Test Tools: Regenerate CSS for the 5.3 branch.

Regenerate CSS vendor prefixes to match the branch's browser support data. This keeps the CSS precommit task and clean-tree check passing.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13402

Reviewed by: desrosj.
Props khokansardar.
See #65993.

#34 @lancewillett
5 weeks ago

  • Resolution → fixed
  • Status accepted → closed

In 63473:

Build/Test Tools: Regenerate CSS for the 5.2 branch.

Regenerate CSS vendor prefixes to match the branch's browser support data. This keeps the CSS precommit task and clean-tree check passing.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13401

Reviewed by: desrosj.
Props khokansardar.
Fixes #65993.

Note: See TracTickets for help on using tickets.