Fix PHP Notice in Translator class #4013

Closed
wants to merge 4 commits into
from

Projects

None yet

3 participants

@xoob

Translator::getTranslatedMessage() causes a PHP Notice due to a change made in 7422c37#L0R398

Notice: Undefined index: en_US in .../Zend/I18n/Translator/Translator.php on line 402

This change adds an isset() to fix this, so that the index is checked as it used to be.

@weierophinney
Zend Framework member

Please add a unit test

@xoob

@weierophinney Done.

$ php run-tests.php I18n
1) ZendTest\I18n\Translator\TranslatorTest::testTranslateNonExistantLocale
Undefined index: es_ES
FAILURES!
Tests: 258, Assertions: 2187, Errors: 1.

$ git checkout bugfix/translator

$ php run-tests.php I18n
OK (258 tests, 2189 assertions)
@DASPRiD
Zend Framework member

This is not really the best solution, as it will try to load messages for that locale over and over again. It makes much more sense to change line 519 (in your codebase) to the following:

$this->messages[$textDomain] = array($locale => null);

And then in getTranslatedMessage() change your new if statement to an !== null comparision.

@xoob

OK, here is an adapted fix.

Instead of initializing with array($locale => null), I opted to check for if (!$messagesLoaded) { at the end of loadMessages() to handle situations where $locale was changed between calls. I've updated the unit test to check for this as well.

Looking forward to your feedback.

@DASPRiD DASPRiD commented on an outdated diff Mar 12, 2013
library/Zend/I18n/Translator/Translator.php
@@ -531,8 +531,12 @@ protected function loadMessages($textDomain, $locale)
|| $this->loadMessagesFromFiles($textDomain, $locale)
);
- if ($messagesLoaded && $cache !== null) {
- $cache->setItem($cacheId, $this->messages[$textDomain][$locale]);
+ if (!$messagesLoaded) {
+ $this->messages[$textDomain][$locale] = null;
+ } else {
+ if ($cache !== null) {
@DASPRiD
DASPRiD Mar 12, 2013

Merge those two lines into } elseif ($ache…

@DASPRiD
Zend Framework member

Looks good like this, except the one thing I noted.

@DASPRiD DASPRiD referenced this pull request Mar 14, 2013
Closed

Update Translator.php #4025

@xoob

@DASPRiD Changed to elseif as requested, in dae382d

@DASPRiD DASPRiD added a commit that closed this pull request Mar 16, 2013
@DASPRiD DASPRiD Merge branch 'hotfix/4013'
Close #4013
e8540e4
@DASPRiD DASPRiD closed this in e8540e4 Mar 16, 2013
@DASPRiD DASPRiD added a commit that referenced this pull request Mar 16, 2013
@DASPRiD DASPRiD Merge branch 'hotfix/4013' into develop
Forward port #4013
8103b37
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@DASPRiD DASPRiD Merge branch 'hotfix/4013'
Close #4013
2a5d02b
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@DASPRiD DASPRiD Merge branch 'hotfix/4013' into develop
Forward port #4013
315dfb4
@gianarb gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@DASPRiD DASPRiD Merge branch 'hotfix/4013' 9092473
@gianarb gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@DASPRiD DASPRiD Merge branch 'hotfix/4013' into develop d46ca96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment