WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 23 months 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 2 years ago.
45769.2.diff (1.6 KB) - added by ocean90 23 months ago.

Download all attachments as: .zip

Change History (14)

@swissspidy
2 years ago

#1 @swissspidy
2 years ago

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

cc @herregroen for review

#2 @herregroen
2 years ago

@swissspidy Patch looks good!

#3 @swissspidy
23 months ago

  • Keywords commit added

#4 @johnbillion
23 months 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
23 months 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
23 months ago

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

#7 @johnbillion
23 months ago

  • Keywords commit added

Tested and looking good!

@ocean90
23 months ago

#8 @ocean90
23 months ago

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

Version 0, edited 23 months ago by ocean90 (next)

#9 @ocean90
23 months 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
23 months ago

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

#11 @ocean90
23 months 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
23 months 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.