-
-
Notifications
You must be signed in to change notification settings - Fork 53
Fixed string translation is invoked before gettext domain is registered. #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed string translation is invoked before gettext domain is registered. #139
Conversation
dominic-ks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep makes sense.
|
I did not add a changelog entry before merging. But I believe that should be generated from the commit log when creating a release? 🤔 — let's cover this in #134 (comment) |
|
Unfortunately this is causing an infinite recursion, but only under limited circumstances; e.g., when accessing WooCommerce Statistics in the admin backend on I'm still debugging why this is only happening in certain situations, but for now here is the relevant infinite recursion part of stack trace: In this case, the entry point seems to be in the enable-media-replace plugin, which might have problematic code on its own (indirectly triggering initialization of authentication in plugins_loaded already) – but regardless of that, the infinite recursion between jwt-auth and l10n.php must not happen. What is happening:
A quick fix would be to just remove the string translations altogether - the strings were not translated previously either. A more sophisticated fix would probably involve checking whether a certain hook was invoked already (e.g., 'init'?) to only translate the strings if that is the case. |
|
Based on https://developer.wordpress.org/apis/hooks/action-reference/, only invoking our The scenario indeed seems to be a special case that is only happening on certain REST routes in wp-admin:
Logging the stack trace in our This is extremely tough to resolve — it essentially means we can only pursue one of these possible paths:
|
|
Also created an issue for enable-media-replace calling |
Problem
WP_DEBUGenabled, WordPress logs the following message for every request/bootstrap:Cause
wp_trigger_error()and checking the stack trace, the cause becomes clear — the plugin tries to translate some strings too early, directly when the code is loaded, before the gettext domain is even registered:Proposed solution
The early translation and collection of translatable strings is completely unnecessary and can be dissolved.
In fact, fixing this is even a micro performance optimization, because
Compatibility
$messagesproperty was private, so not accessible from elsewhere; not even child classes.