#59489 closed defect (bug) (fixed)
Stale core block style cache updates not working on Windows OS
Reported by: | wildworks | Owned by: | joemcgill |
---|---|---|---|
Milestone: | 6.3.2 | Priority: | normal |
Severity: | normal | Version: | 6.3.2 |
Component: | Themes | Keywords: | has-patch fixed-major |
Focuses: | performance | Cc: |
Description
In [56528], The old core block style cache that was stored as a full path is now replaced by a new relative path. However, this cache update doesn't seem to be working on Windows.
As far as I've researched, the problem is related to this code: https://github.com/WordPress/wordpress-develop/blob/e3a612a97c4c45acd7613779a46d86860e76cc65/src/wp-includes/blocks/index.php#L59-L76
This code does not correctly replace the old cache with a relative path on Windows OS, as below:
- If the cache's WordPress version is different from the current WordPress version, try to create a new cache by replacing the full path with a relative path.
- Use the glob function to find the full path of the stylesheet. All path separators in the returned value array are forward slashes:
D:/_local/WordPress/wp6.4/app/public/wp-includes/blocks/archives/editor-rtl.css
D:/_local/WordPress/wp6.4/app/public/wp-includes/blocks/archives/editor-rtl.min.css
- However, the
BLOCKS_PATH
constant on which the substitution is based includes a backslash in the path separator, as shown below:D:\_local\WordPress\wp6.4\app\public/wp-includes/blocks/
- This results in a cache that still has the full path.
- Subsequent style enqueues expect the file path to be relative, so the style will not be registered.
This problem seems to be solved by adding normalization to str_replace()
function as shown below:
$files = array_map( static function ( $file ) { return str_replace( wp_normalize_path( BLOCKS_PATH ), '', $file ); }, $files );
Attachments (2)
Change History (32)
This ticket was mentioned in Slack in #core by wildworks. View the logs.
14 months ago
#5
@
14 months ago
To reproduce this issue and to test the patch, WordPress must be hosted on Windows host OS.
#6
@
13 months ago
I think this issue should be fixed in WordPress 6.4. Otherwise, users developing sites on Windows OS will have old absolute paths cached and will not be able to see the correct block style on the front end.
@
13 months ago
How the Twenty Twenty-Four theme looks on Windows OS when updated to WordPress6.4 Beta1
This ticket was mentioned in Slack in #core by joemcgill. View the logs.
13 months ago
#9
@
13 months ago
- Focuses performance added
- Keywords needs-testing added
- Milestone changed from 6.4 to 6.3.2
#11
@
13 months ago
Test Report
Environment
OS: Windows 11
PHP: 8.0.29
Apache: 2.4.53
WP: 6.4-beta2
Testing
Prior to applying the patch, I was experiencing the problem that no core block CSS was being loaded when a block-based theme was used. After applying the patch, the core block CSS is being loaded.
Great work @wildworks!
This ticket was mentioned in PR #5409 on WordPress/wordpress-develop by joemcgill.
13 months ago
#12
This PR implements the patch provided by wildworks on the original ticket.
Trac ticket: https://core.trac.wordpress.org/ticket/59489
This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.
13 months ago
#14
@
13 months ago
- Keywords needs-unit-tests added
- Owner set to joemcgill
- Status changed from new to assigned
Thanks for confirming @pbiron! I'd like to get some unit tests added for this so the problem doesn't re-surface, but that shouldn't block this from landing in time for 6.3.2 and 6.4-beta3 if needed.
#15
@
13 months ago
I also now done a search thru core code looking for other places where not normalizing BLOCKS_PATH
would cause a problem in Windows. The only other place I can find is https://core.trac.wordpress.org/browser/trunk/src/wp-includes/blocks/index.php?rev=56559#L92 and it's already normalized there.
#16
follow-up:
↓ 18
@
13 months ago
Thanks @pbiron. I did the same search and came to the same conclusion as you. It may be useful to look into normalizing the BLOCKS_PATH
value when it's created, rather than throughout this code, but is not necessary to fix this bug. Can you test the updates I've made to the original patch in PR #5409 and confirm it's still working?
joemcgill commented on PR #5409:
13 months ago
#17
Thanks @felixarntz. I do think that running the test suite on a windows OS would be the best way to uncover this type of issue. I've followed up with @desrosj and may open another ticket to address that issue. Otherwise, we'd need to refactor this function in a way that we could test it with mocked paths.
#18
in reply to:
↑ 16
@
13 months ago
Replying to joemcgill:
It may be useful to look into normalizing the
BLOCKS_PATH
value when it's created, rather than throughout this code, but is not necessary to fix this bug.
I was thinking the same thing, but didn't want to rush into anything like that for fear of breaking something else.
Can you test the updates I've made to the original patch in PR #5409 and confirm it's still working?
I commented on the PR that moving the normalization outside the loop still works...no need to normalize the constant ~500 times ;-)
#19
@
13 months ago
- Keywords commit added; needs-testing needs-unit-tests removed
Given feedback from @pbiron and @flixos90. I'm setting this as ready for commit and will handle getting this in trunk and the 6.3 branch prior to 6.3.2.
Discussion about testing on Windows will need to happen in a separate ticket.
This ticket was mentioned in Slack in #core by joemcgill. View the logs.
13 months ago
#22
@
13 months ago
- Keywords fixed-major added; commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening to backport to the 6.3 branch.
joemcgill commented on PR #5409:
13 months ago
#23
#24
@
13 months ago
@joemcgill Could we not have reused the variable in the line above?
// Normalize BLOCKS_PATH prior to substitution for Windows environments.
$normalized_blocks_path = wp_normalize_path( BLOCKS_PATH );
$files = glob( wp_normalize_path( $normalized_blocks_path . '**/**.css' ) );
#25
@
13 months ago
@spacedmonkey I considered whether we should really normalize the BLOCK_PATH
value when it's created so it's used throughout, but decided against it since other constants like WP_INC
are not following that pattern. There's not really any advantage to using the normalized blocks path in the line above, since we would still need to normalize the '**/ **.css'
part of that glob pattern. I just wanted to make sure we weren't normalizing the same value inside the array_map()
loop. I think any future optimizations to this process could happen in new tickets at this point.
#28
@
13 months ago
Thank you to everyone who worked on this issue.
As a Windows user myself, I understand that differences in path delimiters can cause problems and confusion.
I don't think there are any other major WIndows OS-specific issues in the core so far, but this Gutenberg issue, for example, suggests an issue with normalizing path separators.
P.S. This issue was originally reported on Gutenberg: https://github.com/WordPress/gutenberg/issues/54849