Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#45769 closed defect (bug) (fixed)

Expose JSON translation file paths

Reported by: swissspidy's profile swissspidy Owned by: ocean90's profile 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 6 years ago.
45769.2.diff (1.6 KB) - added by ocean90 6 years ago.

Download all attachments as: .zip

Change History (14)

@swissspidy
6 years ago

#1 @swissspidy
6 years ago

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

cc @herregroen for review

#2 @herregroen
6 years ago

@swissspidy Patch looks good!

#3 @swissspidy
6 years ago

  • Keywords commit added

#4 @johnbillion
6 years 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
6 years 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
6 years ago

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

#7 @johnbillion
6 years ago

  • Keywords commit added

Tested and looking good!

@ocean90
6 years ago

#8 @ocean90
6 years ago

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

Version 0, edited 6 years ago by ocean90 (next)

#9 @ocean90
6 years 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
6 years ago

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

#11 @ocean90
6 years 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
6 years 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.