#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
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.
#5
@
5 weeks ago
- Owner set to
- 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
#6
Trunk fix landed in https://core.trac.wordpress.org/changeset/63445
@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!):
- Revert r63445 on trunk
- Replace the planned
test-css: falsebackports with one PR per affected branch - Each PR should contain only the output of
npm run grunt precommit:css—no workflow changes - Keep the CSS check enabled and confirm the build-process jobs pass on all three operating systems
- 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:
- 5.2 — https://github.com/WordPress/wordpress-develop/pull/13401
- 5.3 — https://github.com/WordPress/wordpress-develop/pull/13402
- 5.4 — https://github.com/WordPress/wordpress-develop/pull/13403
- 5.5 — https://github.com/WordPress/wordpress-develop/pull/13404
- 5.6 — https://github.com/WordPress/wordpress-develop/pull/13405
- 5.7 — https://github.com/WordPress/wordpress-develop/pull/13400
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
@lancewillett commented on PR #13318:
5 weeks ago
#21
@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.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
The
Test Build Processesworkflow now fails for the 5.2 through 5.7 branches.The
precommit:cssstep 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 thegit diff --exit-codestep that follows the build then fails. Since those branches call this reusable workflow at@trunk, this adds atest-cssinput mirroring the existingtest-emojione, letting each branch opt out.It defaults to
true, so no current caller changes behaviour; the affected branches still needtest-css: falseadded alongside their existingtest-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:cssstep and drafting the workflow input. All changes were reviewed and validated by me.