Make WordPress Core

Opened 10 months ago

Closed 9 months ago

Last modified 7 months ago

#58711 closed defect (bug) (fixed)

register_core_block_style_handles() does not enqueue styles on Windows OS

Reported by: wildworks's profile wildworks Owned by: audrasjb's profile audrasjb
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.3
Component: Editor Keywords: has-testing-info has-patch commit
Focuses: Cc:

Description

Originally reported in the Gutenberg repository: https://github.com/WordPress/gutenberg/issues/52199

This problem only occurs when WordPress itself is running on the Windows host OS; when the WordPress repository is running via npm script, WordPress is running on Docker (on Ubuntu), so this problem should not be reproducible. Running the built source or a released package (such as the Beta3 version) in a local environment on a Windows OS would reproduce this problem.

This function uses the glob function to locate CSS files under wp-includes/blocks. In this case, $files variable has the following value on Windows OS:

array(
  // ... 
  "D:\_local\test\wp6.3\app\public\wp-includes\blocks/archives/editor.min.css"
  "D:\_local\test\wp6.3\app\public\wp-includes\blocks/archives/style.min.css"
  // ... 
);

Note the mix of forward slash and backslash as path separators.

On the other hand, the $path in $register_style function has the following value:

"D:\_local\test\wp6.3\app\public/wp-includes/blocks/archives/editor.min.css"
"D:\_local\test\wp6.3\app\public/wp-includes/blocks/archives/style.min.css"

Comparing with the value that the $files array has, you can see that some path separators are different. As a result, the check by in_array( $path, $files, true ) will be false and the style will not be enqueued.

We probably need to unify the path separator characters in some way, as I expect this problem to occur on all Windows OS.

Attachments (2)

patch_58711.diff (1.3 KB) - added by wildworks 10 months ago.
patch_58711.diff
58711-without-array_map.diff (1.0 KB) - added by costdev 9 months ago.
Updated patch_58711.diff without array_map().

Download all attachments as: .zip

Change History (20)

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


10 months ago

#2 @costdev
10 months ago

  • Component changed from General to Editor
  • Keywords needs-testing has-testing-info added
  • Milestone changed from Awaiting Review to 6.3
  • Version set to trunk

Hi @wildworks, thanks for opening this ticket!

  • As this function was introduced during the 6.3 cycle in [56044], milestoning for 6.3 for visibility and setting the Version property to trunk.
  • Adding needs-testing so this can be reproduced by a member of the Test team. Testing instructions can be found here.
  • Note to testers: You may need to change this line to $files = false; // Just for testing..
  • Pinging @spacedmonkey who worked on this one. Looks like wp_normalize_path() is needed here.
Last edited 9 months ago by costdev (previous) (diff)

@wildworks
10 months ago

patch_58711.diff

#3 @wildworks
10 months ago

I have created a patch to resolve this issue. (This is my first time creating a diff patch file, so sorry if I'm wrong) At least in my environment, this patch seems to fix the problem.

@costdev
9 months ago

Updated patch_58711.diff without array_map().

#4 @costdev
9 months ago

  • Keywords has-patch added

Myself and @wildworks discussed the issue and did some additional testing.

I've submitted an updated patch 58711-without-array_map.diff which uses wp_normalize_path() on the path passed to glob(), rather than running array_map() on the result.

Last edited 9 months ago by costdev (previous) (diff)

#6 @kafleg
9 months ago

Test Report

This patch worked for me.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58711/58711-without-array_map.diff

Environment

  • OS: Windows 11
  • Web Server: Laragon Local Server
  • PHP: 8.1.2
  • WordPress: 6.3-Beta-3
  • Browser: Edge and Chrome
  • Theme: Twenty Twenty Three
  • Active Plugins:
    • None

Actual Results

  • ✅ Issue resolved with patch.

#7 @costdev
9 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/raw-attachment/ticket/58711/58711-without-array_map.diff

Steps to Reproduce or Test

  1. In a Windows environment, ensure that the Navigation block is in the template.
  2. Create a post with some blocks.
  3. 🐞 View the post on the frontend.
  4. Apply the patch.
  5. Change this line to $files = false; // Just for testing.
  6. Refresh the post on the frontend.

Expected Results

When reproducing a bug:

  • ❌ The Navigation block and others have incorrect styling.

When testing a patch to validate it works as expected:

  • ✅ The Navigation block and others have the correct styling.

Environment

  • WordPress: 6.3-beta3
  • OS: Windows
  • PHP: 8.1.9
  • Server: nginx/1.16.0 (using Local)
  • Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.9)
  • Browser: Chrome 114.0.0.0 (Windows 10/11)
  • Theme: Twenty Twenty-Three 1.1
  • MU-Plugins: None activated
  • Plugins: WordPress Beta Tester 3.5.0

Actual Results

When reproducing a bug/defect:

  • ❌ The Navigation block and others had incorrect styling.

When testing the bugfix patch:

  • ✅ The Navigation block and others have the correct styling.
Last edited 9 months ago by costdev (previous) (diff)

#8 @wildworks
9 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58711/58711-without-array_map.diff

Environment

  • OS: Windows 11
  • Web Server: Nginx
  • PHP: 8.1.9
  • WordPress: WordPress 6.3-beta3
  • Browser: Chrome 114.0.5735.199, Firefox 115.0
  • Theme: Twenty Twenty Two, Twenty Twenty Three
  • Active Plugins: WordPress Beta Tester 3.5.0

Actual Results

  • ✅ Issue resolved with patch.

#9 @costdev
9 months ago

  • Keywords commit added; needs-testing removed

With three testers reporting that 58711-without-array_map.diff resolves this Windows-only issue in their environments, I'm removing needs-testing and adding this for commit consideration.

#10 @mukesh27
9 months ago

@costdev could you open PR so we can check performance number?

This ticket was mentioned in PR #4792 on WordPress/wordpress-develop by @costdev.


9 months ago
#11

Previously, this function did not normalize paths before using glob() or registering styles. This led to a mixture of forward-slashes and back-slashes in Windows environments.

This change uses wp_normalize_path() to ensure Windows compatibility.

#12 @costdev
9 months ago

@mukesh27 See PR 4792.

#13 @audrasjb
9 months ago

  • Owner set to audrasjb
  • Resolution set to fixed
  • Status changed from new to closed

In 56136:

Editor: Normalize paths in register_core_block_style_handles().

Previously, this function did not normalize paths before using glob() or registering styles. This led to a mixture of forward-slashes and back-slashes in
Windows environments.

This change uses wp_normalize_path() to ensure Windows compatibility.

Props wildworks, costdev, kafleg, mukesh27.
Fixes #58711.

#14 follow-up: @spacedmonkey
9 months ago

@audrasjb Similar code has existed in core for a while. See https://github.com/WordPress/wordpress-develop/commit/f034bc832eb091079e4130f6a206b4f496766796. Does this change need to be backported?

#15 in reply to: ↑ 14 @audrasjb
9 months ago

Replying to spacedmonkey:

@audrasjb Similar code has existed in core for a while. See https://github.com/WordPress/wordpress-develop/commit/f034bc832eb091079e4130f6a206b4f496766796. Does this change need to be backported?

Need to be backported to what? Gutenberg? Sorry but could you please clarify?

This ticket was mentioned in Slack in #core-test by costdev. View the logs.


9 months ago

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


7 months ago

Note: See TracTickets for help on using tickets.