Fix block type registration bugs on Windows by correctly normalizing paths #8417

felixarntz · 2025-02-26T21:51:17Z

This PR aims to apply wp_normalize_path() correctly throughout block type registration.
The accompanying tests verify the fixes:
- In 277eb72, I committed only tests, to show the failures. See e.g. https://github.com/WordPress/wordpress-develop/actions/runs/13554067977/job/37884414701?pr=8417#step:14:895
- In 67badb1, I committed the fixes, to show that they address the failures. See e.g. https://github.com/WordPress/wordpress-develop/actions/runs/13554252283/job/37885015186?pr=8417
For WP_Block_Metadata_Registry, the approach taken is that all public methods have their inputs normalized, while the private methods expect already normalized paths (which has been included in the documentation and should be fine since those methods are private to the class).

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

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2025-02-26T21:51:41Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props flixos90, joemcgill, gziolo.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2025-02-26T22:04:01Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

joemcgill

I'd like to see a real user test of this to confirm that the assumptions in the PHPUnit test are correct, but this looks right to me.

joemcgill · 2025-02-26T22:41:21Z

src/wp-includes/class-wp-block-metadata-registry.php

 	 */
 	private static function find_collection_path( $file_or_folder ) {
 		if ( empty( $file_or_folder ) ) {
 			return null;
 		}

 		// Check the last matched collection first, since block registration usually happens in batches per plugin or theme.
-		$path = wp_normalize_path( rtrim( $file_or_folder, '/' ) );
+		$path = rtrim( $file_or_folder, '/' );


Makes sense, because normalization is happening in the public method ::get_metadata() before passing the value to this and the also private ::default_identifier_callback() methods. Thanks for clarifying in the docs for both ✨

gziolo

That was a quick follow-up. It all looks good to me. I don't have a Windows machine to verify, but the included tests look fine to me.

For WP_Block_Metadata_Registry, the approach taken is that all public methods have their inputs normalized, while the private methods expect already normalized paths (which has been included in the documentation and should be fine since those methods are private to the class).

I remember that in WordPress 6.4 there was a new function introduced get_block_asset_url as a performant way to calculate the URL for the block's asset (JS or CSS). Example usage:

wordpress-develop/src/wp-includes/blocks.php

Lines 238 to 239 in b45e379

    
           $script_path_norm    = wp_normalize_path( realpath( $path . '/' . $script_path ) ); 
        
           $script_uri          = get_block_asset_url( $script_path_norm );

It operates on the normalized URLs so that would be similar to how the private methods in the class are handled. It makes sense to me in this particular case that public methods related to blocks at this level of abstraction take any path and normalize everything internally.

Out of curiosity, do you know why in the get_block_asset_url there is realpath used in a few places, example:

wordpress-develop/src/wp-includes/blocks.php

Lines 95 to 99 in b45e379

    
           // Path needs to be normalized to work in Windows env. 
        
           static $wpinc_path_norm = ''; 
        
           if ( ! $wpinc_path_norm ) { 
        
           	$wpinc_path_norm = wp_normalize_path( realpath( ABSPATH . WPINC ) ); 
        
           }

felixarntz · 2025-02-27T18:42:51Z

@gziolo

Out of curiosity, do you know why in the get_block_asset_url there is realpath used in a few places, example:

wordpress-develop/src/wp-includes/blocks.php

Lines 95 to 99 in b45e379

// Path needs to be normalized to work in Windows env.

static $wpinc_path_norm = '';

if ( ! $wpinc_path_norm ) {

$wpinc_path_norm = wp_normalize_path( realpath( ABSPATH . WPINC ) );

}

Based on git blame, you may be the best person to answer this 😂
See 573581f and https://core.trac.wordpress.org/ticket/55311

cc @audrasjb

Based on that ticket, that same bug would currently still apply to register_block_type_from_metadata() and WP_Block_Metadata_Registry. Do we need to use realpath() on those base paths for those functions too (e.g. ABSPATH . WPINC, WP_CONTENT_DIR, WPMU_PLUGIN_DIR, etc.)? Or is this only relevant for the function handled in the ticket because those paths are turned into URLs?

gziolo · 2025-02-28T07:00:33Z

Based on git blame, you may be the best person to answer this 😂
See 573581f and https://core.trac.wordpress.org/ticket/55311

Intriguing discovery 🙈😃

It looks like this particular issue existed when using symlinks:

For us, we are running WP inside docker, and our WP core files exist at /wordpress-versions/5.9/ in the container, but the web root is at /httpdocs,

In addition, there were mentions in https://core.trac.wordpress.org/ticket/62140 of prior limitations with block metadata collections when using symlinks. However, this was likely addressed in https://core.trac.wordpress.org/changeset/59730.

felixarntz · 2025-02-28T17:13:58Z

It looks like this particular issue existed when using symlinks:

The thing is I'm not sure whether that problem is relevant here or not. Of course, we could just go the safe route and include realpath() calls everywhere, since I don't see any harm in adding them (except that, if they are unnecessary, we add code clutter; but there would not be any notable adverse effects on the actual functionality or performance).

Should we maybe just add them to be safe? Or should we commit this as is and see if any other bug reports come up in the future?

gziolo · 2025-03-03T08:44:40Z

Should we maybe just add them to be safe? Or should we commit this as is and see if any other bug reports come up in the future?

Let's proceed as is and collect more details before adding additional function calls.

Add tests to show broken behavior with Windows paths.

277eb72

Fix path normalization bugs for Windows.

67badb1

felixarntz requested review from gziolo and joemcgill February 26, 2025 22:26

felixarntz changed the title ~~Add tests to show broken behavior with Windows paths.~~ Fix block type registration bugs on Windows by correctly normalizing paths Feb 26, 2025

Ensure ABSPATH . WPINC is normalized.

a3b0a59

joemcgill approved these changes Feb 26, 2025

View reviewed changes

gziolo approved these changes Feb 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix block type registration bugs on Windows by correctly normalizing paths #8417

Fix block type registration bugs on Windows by correctly normalizing paths #8417

felixarntz commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025

joemcgill left a comment

joemcgill Feb 26, 2025

gziolo left a comment

felixarntz commented Feb 27, 2025 •

edited

Loading

gziolo commented Feb 28, 2025 •

edited

Loading

felixarntz commented Feb 28, 2025

gziolo commented Mar 3, 2025 •

edited

Loading

	$script_path_norm = wp_normalize_path( realpath( $path . '/' . $script_path ) );
	$script_uri = get_block_asset_url( $script_path_norm );

	// Path needs to be normalized to work in Windows env.
	static $wpinc_path_norm = '';
	if ( ! $wpinc_path_norm ) {
	$wpinc_path_norm = wp_normalize_path( realpath( ABSPATH . WPINC ) );
	}

Fix block type registration bugs on Windows by correctly normalizing paths #8417

Are you sure you want to change the base?

Fix block type registration bugs on Windows by correctly normalizing paths #8417

Conversation

felixarntz commented Feb 26, 2025 • edited Loading

github-actions bot commented Feb 26, 2025 • edited Loading

github-actions bot commented Feb 26, 2025

Test using WordPress Playground

Some things to be aware of

joemcgill left a comment

Choose a reason for hiding this comment

joemcgill Feb 26, 2025

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

felixarntz commented Feb 27, 2025 • edited Loading

gziolo commented Feb 28, 2025 • edited Loading

felixarntz commented Feb 28, 2025

gziolo commented Mar 3, 2025 • edited Loading

felixarntz commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading

felixarntz commented Feb 27, 2025 •

edited

Loading

gziolo commented Feb 28, 2025 •

edited

Loading

gziolo commented Mar 3, 2025 •

edited

Loading