[wp-trac] [WordPress Trac] #21319: is_textdomain_loaded() returns true even if there are no translations for the domain

[WordPress Trac]

#21319: is_textdomain_loaded() returns true even if there are no translations for
the domain
------------------------------+-------------------------
 Reporter:  nacin             |       Owner:  nbachiyski
     Type:  defect (bug)      |      Status:  accepted
 Priority:  normal            |   Milestone:  3.5
Component:  I18N              |     Version:  3.0
 Severity:  normal            |  Resolution:
 Keywords:  has-patch commit  |
------------------------------+-------------------------

Comment (by nacin):

 Replying to [comment:3 nbachiyski]:
 > If you, for whatever reason, need a function to check if a textdomain
 loaded actual MO file, we could have another function for that, but I
 don't see a use case for it for now.

 > I am not a big fan of the micro-optimization.  First, it makes
 {{{get_translations_for_domain()}}} harder to read and inverses the logic
 there. The most common case: {{{return $l10n[ $domain ];}}} is buried in
 the middle of the function deeper intended than the rest of the function.
 Second, we usually have just a handful of textdomains and the noop class
 is very small, so I don't see the point.

 It's actually not a micro-optimization. It was originally suggested as
 such, but it's necessary if we were to have made this change. No longer
 would we be storing NOOP_Translations in $l10n, which means we would have
 instantiated a new NOOP_Translations for every call to
 get_translations_for_domain() — which can occur hundreds or thousands of
 times. The static variable was the only sane way to do that.

 Seems like a function to check if translations are loaded can check `!
 isset( $l10n[ $domain ] ) || $l10n[ $domain ] instanceof
 NOOP_Translations`. We do actually have a use case for this — in WP_Theme,
 there is a load_textdomain() method that is supposed to return true if we
 have translations, and false if we don't, to avoid unnecessary translate()
 calls.

 The other issue — highlighted by the WP_Theme method — is that the
 load_textdomain() function (and its plugin/theme/child theme friends) all
 return `true` if a MO file was loaded, and false if it was not. So
 shouldn't is_textdomain_loaded() do the same? I understand the point; it
 just seems inconsistent.

 It's also worth nothing that since "the purpose of the function is to
 prevent unwanted expensive double loading of translation files," this
 change wouldn't break that. It would also fix an issue where if a double-
 loading were to occur with the first one not finding a MO file, it would
 still trigger an expensive merge_with(). (The `isset( $l10n[ $domain ] )`
 in load_textdomain() should probably also check for `! instanceof
 NOOP_Translations`.)

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/21319#comment:4>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software