Skip to content
This repository

[Form] [FormElementManager] don't overwrite form factory if already set #3735

Merged
merged 1 commit into from about 1 year ago

3 participants

Rafi Adnan Ralph Schindler Matthew Weier O'Phinney
Rafi Adnan

Don't overwrite factory in case other initializers are called beforehand that set the form factory, or attach an input filter factory to it.

Ralph Schindler

Unit test?

Matthew Weier O'Phinney weierophinney commented on the diff February 19, 2013
library/Zend/Form/FormElementManager.php
@@ -84,7 +84,7 @@ public function __construct(ConfigInterface $configuration = null)
84 84
     public function injectFactory($element)
85 85
     {
86 86
         if ($element instanceof FormFactoryAwareInterface) {
87  
-            $element->setFormFactory(new Factory($this));
  87
+            $element->getFormFactory()->setFormElementManager($this);
2
Matthew Weier O'Phinney Owner

These are actually synonymous -- getFormFactory() will create a form factory instance internally. The old code is actually more performant than your change as it bypasses the getter and immediately sets a new instance that has the element manager composed. (The reason no unit tests fail currently is because we actually have a test that verifies that the form element manager is identical in the two objects.)

Can you indicate why you would want to alter this? A unit test detailing the use case would make the most sense, as right now, I'm unclear what the purpose of the change is.

Matthew Weier O'Phinney Owner

Actually, your subject line details it: you want to inject the element manager into an existing factory, if present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Matthew Weier O'Phinney weierophinney referenced this pull request from a commit February 19, 2013
Matthew Weier O'Phinney [#3735] Added unit test
- Test demonstrates that factory composed by object implementing
  FormFactoryAwareInterface is not overwritten, but *is* injected with
  FormElementManager instance.
c24fb6f
Matthew Weier O'Phinney weierophinney merged commit 6ce846e into from February 19, 2013
Matthew Weier O'Phinney weierophinney closed this February 19, 2013
Matthew Weier O'Phinney

Added a unit test on merge.

Deleted user Unknown referenced this pull request from a commit February 19, 2013
Matthew Weier O'Phinney [#3735] Added unit test
- Test demonstrates that factory composed by object implementing
  FormFactoryAwareInterface is not overwritten, but *is* injected with
  FormElementManager instance.
0901c55
Deleted user Unknown referenced this pull request from a commit February 19, 2013
Matthew Weier O'Phinney Merge branch 'hotfix/3735'
Close #3735
f9f526b
Deleted user Unknown referenced this pull request from a commit February 19, 2013
Matthew Weier O'Phinney Merge branch 'hotfix/3735' into develop
Forward port #3735
86a58c0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Feb 08, 2013
Rafi Adnan don't overwrite form factory if already set 6ce846e
This page is out of date. Refresh to see the latest.
2  library/Zend/Form/FormElementManager.php
@@ -84,7 +84,7 @@ public function __construct(ConfigInterface $configuration = null)
84 84
     public function injectFactory($element)
85 85
     {
86 86
         if ($element instanceof FormFactoryAwareInterface) {
87  
-            $element->setFormFactory(new Factory($this));
  87
+            $element->getFormFactory()->setFormElementManager($this);
88 88
         }
89 89
     }
90 90
 
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.