ZF-11112: Zend_Translate_Adapter emits notice when 'content' option is not provided to addTranslation()

Issue Type:
Improvement
Created:
2011-02-25T01:35:52.000+0000
Last Updated:
2011-12-02T23:50:38.000+0000
Status:
Resolved
Fix version(s):
  • 1.11.11 (29/Sep/11)
  • Next Major Release ()
Reporter:
Ronny Deter (ronnydeter)
Assignee:
Adam Lundrigan (adamlundrigan)
Tags:
  • Zend_Translate
Related issues:
Attachments:

Description

In Line 227 of abstract Zend_Translate_Adapter the options array evaluated of the key 'content'

if (!($options['content'] instanceof Zend_Translate) && !($options['content'] instanceof Zend_Translate_Adapter)) { ...
}

When the key is not defined as index in options, php throws a notice

Notice: Undefined index: content in /home/ronny/svn/trunk/library/vendor/sources/ZendFramework-1.11.1-minimal/library/Zend/Translate/Adapter.php on line 227

better is to check if the key exists

if (array_key_exists($options['content']) && !($options['content'] instanceof Zend_Translate) && !($options['content'] instanceof Zend_Translate_Adapter)) { ...
}

Comments

Posted by Thomas Weidner (thomas) on 2011-02-25T07:49:25.000+0000

'content' must always be given by the adapter as this is the translation content itself.

It is impossible that 'content' is not given because this would mean that you have nothing to translate. An empty translation object is not allowed as it would always return a warning as no single language would exist.

The adapter must in this case throw an exception.

Posted by Adam Lundrigan (adamlundrigan) on 2011-05-01T03:23:19.000+0000

It's been more than a month with no response from original reporter, so I have marked issue as resolved, not an issue.

Just a thought: Would it be a good idea to enforce the requirement of supplying a 'content' key? ie:


Index: library/Zend/Translate/Adapter.php
===================================================================
--- library/Zend/Translate/Adapter.php  (revision 23914)
+++ library/Zend/Translate/Adapter.php  (working copy)
@@ -212,6 +212,11 @@
         } else if (!is_array($options)) {
             $options = array('content' => $options);
         }
+
+        if (!isset($options['content']) || empty($options['content'])) {
+            require_once 'Zend/Translate/Exception.php';
+            throw new Zend_Translate_Exception("Required option 'content' is missing");
+        }

         $originate = null;
         if (!empty($options['locale'])) {

Posted by Thomas Weidner (thomas) on 2011-05-01T06:57:14.000+0000

Reopening issue and changing to improvement

Posted by Adam Lundrigan (adamlundrigan) on 2011-05-03T13:58:40.000+0000

Fixed in trunk r23962

Posted by Thomas Weidner (thomas) on 2011-05-03T14:25:04.000+0000

Adam, thnx for your fix

Please do actually not close I18n related issues as I18n is being reworked. This is intentionally Thnx

Posted by Adam Lundrigan (adamlundrigan) on 2011-06-07T23:01:43.000+0000

Merged to release-1.11 in r24126 Leaving issue open-unresolved and assigned to Thomas Weidner, at his request

Posted by Thomas Weidner (thomas) on 2011-08-25T20:16:25.000+0000

Fixed in ZF2 with GH-278 Attached to Adam as he provided the correct patch

Posted by Curtis Kaffer (kaffer) on 2011-12-02T23:50:38.000+0000

By requiring 'content' you seem to be insisting that custom adapters will use it. This is not always the case. I have an adapter that does real-time translation via a web service. It isn't relying on local files.

I believe it should be up to the individual adapter to decide if a certain option is required.