WordPress.org

Make WordPress Core

Opened 3 weeks ago

Closed 10 days ago

#45769 closed defect (bug) (fixed)

Expose JSON translation file paths

Reported by: swissspidy Owned by: ocean90
Milestone: 5.0.3 Priority: normal
Severity: normal Version: 5.0.2
Component: I18N Keywords: has-patch commit
Focuses: Cc:

Description

As noted by @johnbillion in 45425#comment:5, the potential translation file paths aren't exposed in load_script_textdomain() / load_script_translations().

The determined paths are only passed to load_script_translations() when the files exist. If they don't, load_script_translations() only receives false.

This way it's impossible to hook into this for proper debugging like in Query Monitor, or adding language fallbacks like in the Preferred Languages feature plugin.

Attachments (2)

45769.diff (1.6 KB) - added by swissspidy 3 weeks ago.
45769.2.diff (1.6 KB) - added by ocean90 11 days ago.

Download all attachments as: .zip

Change History (14)

@swissspidy
3 weeks ago

#1 @swissspidy
3 weeks ago

  • Milestone changed from Awaiting Review to 5.0.3
  • Type changed from enhancement to defect (bug)

cc @herregroen for review

#2 @herregroen
2 weeks ago

@swissspidy Patch looks good!

#3 @swissspidy
2 weeks ago

  • Keywords commit added

#4 @johnbillion
2 weeks ago

  • Keywords commit removed

I can't test this right now but it looks like this causes all the processing in load_script_textdomain() to run three times if there are no translation files present. At the moment, the file_exists() checks guard against this.

#5 @swissspidy
2 weeks ago

I don't see how this would run all of that three times. 🤔It just goes through the whole function and calls load_script_translations() for all cases instead of returning early.

#6 @desrosj
13 days ago

@johnbillion have you had a chance to test this? It would be great to get this into 5.0.3.

#7 @johnbillion
12 days ago

  • Keywords commit added

Tested and looking good!

@ocean90
11 days ago

#8 @ocean90
11 days ago

I think we shouldn't call load_script_translations() if $path wasn't set like before, see 45769.2.diff.

Last edited 11 days ago by ocean90 (previous) (diff)

#9 @ocean90
10 days ago

@swissspidy Can you confirm that 45769.2.diff would still work with whatever you had in mind for making it compatible with Preferred Languages?

#10 @swissspidy
10 days ago

Just tested and it works like a charm with the changes made to Preferred Languages! Feel free to commit :-)

#11 @ocean90
10 days ago

In 44418:

I18n: Expose JSON translation file paths in load_script_textdomain().

Removes file_exist() checks before calling load_script_translations() to let the determined paths be passed to load_script_translations() which provides its own file check and the possibility to filter the path.

Props swissspidy, johnbillion, ocean90.
See #45769.

#12 @ocean90
10 days ago

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

In 44419:

I18N: Expose JSON translation file paths in load_script_textdomain().

Removes file_exist() checks before calling load_script_translations() to let the determined paths be passed to load_script_translations() which provides its own file check and the possibility to filter the path.

Merge of [44418] to the 5.0 branch.

Props swissspidy, johnbillion, ocean90.
Fixes #45769.

Note: See TracTickets for help on using tickets.