Make WordPress Core

Opened 2 years ago

Last modified 8 months ago

#54797 assigned enhancement

Allow languages path in register_block_type

Reported by: rahe's profile Rahe Owned by: gziolo's profile gziolo
Milestone: Future Release Priority: normal
Severity: normal Version: 5.5
Component: I18N Keywords: has-patch needs-refresh needs-unit-tests
Focuses: Cc:

Description

Hello,

On register_block_type and block.json right now the languages folder path is not handled.
This calls the register_block_script_handle function that just do :

<?php
        if ( ! empty( $metadata['textdomain'] ) && in_array( 'wp-i18n', $script_dependencies, true ) ) {
                wp_set_script_translations( $script_handle, $metadata['textdomain'] );
        }

Since it's pretty common to change where the languages are stored into a folder, I think we need to :

  • Add to the block.json an entry to specify the languages folder
  • Impact this part of code and add maybe add filters

Nicolas,

Attachments (2)

54797.diff (1.2 KB) - added by gziolo 2 years ago.
54797-domainpath.diff (1.2 KB) - added by gziolo 2 years ago.
Domain Path handling in block.json

Download all attachments as: .zip

Change History (31)

#1 @gziolo
2 years ago

We discussed this issue also on WordPress Slack (link requires registration at https://make.wordpress.org/chat):

https://wordpress.slack.com/archives/C02QB2JS7/p1641920214075100

There is also an issue opened in the Gutenberg project:

https://github.com/WordPress/gutenberg/issues/34478

There are two issues identified:

  1. When providing file paths to scripts (editorScript, script or viewScript), when there is a trailing ./ included then there is wrong md5 generated for the file and it doesn't match with the file generated in the translations folder.
  2. When a plugin provides a custom path to the translations folder, example: languages, it doesn't get passed to wp_set_script_translations called inside the register_block_script_handle helper function.

The first issue can be solved by always removing the trailing ./ from the file path passed in block.json.

The second issue can be tackled in many ways. One possibility is what @Rahe proposed in this ticket. An alernative to consider is to detect whether a script is registered inside a plugin (vs core block) and pass the custom path based on the setting read from the domain path set in the plugin metadata. More about that in https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#domain-path.

This ticket was mentioned in Slack in #core-editor by gziolo. View the logs.


2 years ago

#3 @gziolo
2 years ago

  • Owner set to gziolo
  • Status changed from new to assigned

An alernative to consider is to detect whether a script is registered inside a plugin (vs core block) and pass the custom path based on the setting read from the domain path set in the plugin metadata. More about that in https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#domain-path.

It might be challenging to read the domain path provided in the plugin's metadata because get_plugin_data() is defined only in the admin but not on the frontend.

@gziolo
2 years ago

#4 @gziolo
2 years ago

  • Keywords has-patch has-unit-tests added
  • Milestone changed from Awaiting Review to 5.9.1
  • Type changed from feature request to defect (bug)

When providing file paths to scripts (editorScript, script or viewScript), when there is a trailing ./ included then there is wrong md5 generated for the file and it doesn't match with the file generated in the translations folder.

This part is ready with the patch attached.

#5 @Rahe
2 years ago

Hello,

Great!
A suggestion maybe can we just use ltrim('./', $path); instead of substr+strpos ?

For the plugin/theme header, this is read in the frontoffice too for the "classic" translations no ?

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


2 years ago

#7 @audrasjb
2 years ago

  • Milestone changed from 5.9.1 to 6.0

Hello, it looks like the issue wasn't introduced in 5.9 development cycle, so let's move this ticket to milestone 6.0 as 5.9.1 is dedicated to fix regressions reported against 5.9 :)

#8 @gziolo
2 years ago

I considered ltrim('./', $path); as a first option, but the way it works is a bit confusing. It removes all matching characters from the beginning of the string rather than the exact match we want here ./. Example: ./../file.js becomes file.jswhen you run the following code:

<?php
echo ltrim( './../file.js', './');

#9 @gziolo
2 years ago

  • Version changed from 5.9 to trunk

#10 @gziolo
2 years ago

In 52699:

I18n: Standardize the script paths for blocks

When providing file paths to scripts (editorScript, script or viewScript), when there is a trailing ./ included then there was a different md5 generated for the file that didn't match the one used with the file generated in the translations folder.

Props Rahe.
See #54797.

This ticket was mentioned in Slack in #core-i18n by gziolo. View the logs.


2 years ago

#12 @gziolo
2 years ago

When a plugin provides a custom path to the translations folder, example: languages, it doesn't get passed to wp_set_script_translations called inside the register_block_script_handle helper function.

I added a new patch that allows setting the equivalent of Domain Path like for the plugin. Exactly the same rules apply:

The domain path defines the location for a plugin’s translation. This has a few uses, notably so that WordPress knows where to find the translation even when the plugin is disabled. This defaults to the folder in which your plugin is found.

For example, if the translation is located in a folder called languages within your plugin, then the Domain Path is /languages and must be written with the first slash.

block.json

"textdomain": "my-plugin",
"domainpath" "/languages",

We will have to include similar documentation in Gutenberg in https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/block-api/block-metadata.md with the note that this API is available starting from WordPress 6.0.

This ticket was mentioned in Slack in #core-i18n by gziolo. View the logs.


2 years ago

@gziolo
2 years ago

Domain Path handling in block.json

#14 @ocean90
2 years ago

#53097 was marked as a duplicate.

This ticket was mentioned in Slack in #core-editor by gziolo. View the logs.


2 years ago

#16 @costdev
2 years ago

  • Version changed from trunk to 5.5

#17 @costdev
2 years ago

  • Milestone changed from 6.0 to 6.1

With 6.0 RC1 starting soon, I'm moving this to the 6.1 milestone.

This ticket was mentioned in Slack in #cli by rahe. View the logs.


22 months ago

#19 @audrasjb
20 months ago

  • Milestone changed from 6.1 to 6.2

With WP 6.1 RC 1 scheduled today (Oct 11, 2022), there is not much time left to address this ticket. Let's move it to the next milestone.

Ps: if you were about to send a patch and if you feel it is realistic to commit it in the next one or two hours, please feel free to move this ticket back to milestone 6.1.

#20 @felipeelia
19 months ago

Hi there!

What I'm sharing here is the result of some time debugging a problem that although not strictly the same as outlined in the ticket seems to be so related that doesn't worth a new ticket IMHO. Let me know if you all prefer it in a new ticket and I can create that. I also want to ask you to excuse me if I missed something obvious here :)

I got here trying to translate a JS file added via block.json's editorScript. In my case, it has a long relative path, like file:/../../../../../dist/js/facets-meta-block-script.js

That seems to be generating a discrepancy between the md5 value while creating the .json file and the md5 value actually expected when loading translations. Here is some code to (hopefully) clarify it a bit:

File creation

It seems to use the "real" file name here. In this case, dist/js/facets-meta-block-script.js.

File detection

Due to the way we are generating the src value here, it becomes https://<domain>/wp-content/plugins/elasticpress/assets/js/blocks/facets/taxonomy/../../../../../dist/js/facets-block-script.js. That assets/js/blocks/facets/taxonomy is where the block.json is located.

Then in load_script_textdomain, the value used to get an md5 hash here is actually assets/js/blocks/facets/meta/../../../../../dist/js/facets-meta-block-script.js and not dist/js/facets-meta-block-script.js as used during the generation step.

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


15 months ago

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


15 months ago

#23 @swissspidy
15 months ago

  • Milestone changed from 6.2 to 6.3

#24 @swissspidy
12 months ago

  • Keywords needs-refresh needs-unit-tests added; has-unit-tests removed

Patch needs an update to pass an empty string instead of null by default. Also needs unit tests.

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


11 months ago

#26 @oglekler
11 months ago

This ticket was discussed in the recent bug scrub and because this patch needs a little bit of an additional work, it left in the current 6.3 Milestone for now.

Props to @Clorith

#27 @swissspidy
11 months ago

  • Milestone changed from 6.3 to 6.4
  • Type changed from defect (bug) to enhancement

This is more an enhancement and there has been no activity; bumping to 6.4

#28 @oglekler
9 months ago

@gziolo can you procced with what was asked in the #comment:24?

#29 @oglekler
8 months ago

  • Milestone changed from 6.4 to Future Release

There is no progress in 11 months, and we have 9 days before Beta 1, so I am moving it into Future release. When the patch will be ready, it can go into the next available milestone.

Note: See TracTickets for help on using tickets.