Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Improved previous fix for ZF2-558. #2438

Closed
wants to merge 10 commits into from

3 participants

Tim Lieberman Michaël Gallego Matthew Weier O'Phinney
Tim Lieberman

Previous fix made implicit assumptions that could cause issues in subclasses of Element[Select|MultiCheckbox]. This fix dispenses with those assumptions so subclasses that override getValidator() and attach validators that are neither InArray or Explode won't break. For instance, DoctrineORMModule\Form\Element\DoctrineEntity was broken by the original fix (294a7d1)

Tim Lieberman Improved previous fix for ZF2-558. Previous fix made implicit assumpt…
…ions that could cause issues in subclasses of Element\[Select|MultiCheckbox]
d5f24fc
Michaël Gallego

Speaking about DoctrineEntity element, I made a PR to fix the error (but it has not been merged yet).

Anyway I think you're right, it was wrong to assume the validator was always InArray.

library/Zend/Form/Element/MultiCheckbox.php
@@ -64,7 +64,7 @@ public function setValueOptions(array $options)
$this->valueOptions = $options;
// Update InArray validator haystack
- if (!is_null($this->validator)) {
+ if (!is_null($this->validator) && $this->validator instanceof ExplodeValidator) {
Matthew Weier O'Phinney Owner

Simply doing the instanceof check would suffice here:

if ($this->validator instanceof ExplodeValidator) {
Matthew Weier O'Phinney Owner

Also, update the preceding comment, as it's now invalid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Form/Element/Select.php
@@ -83,8 +83,15 @@ public function setValueOptions(array $options)
// Update InArrayValidator validator haystack
if (!is_null($this->validator)) {
- $validator = $this->validator instanceof InArrayValidator ? $this->validator : $this->validator->getValidator();
- $validator->setHaystack($this->getValueOptionsValues());
+ if ($this->validator instanceof InArrayValidator){
+ $validator = $this->validator;
+ }
+ if ($this->validator instanceof ExplodeValidator){
+ $validator = $this->validator->getValidator();
+ }
+ if (!empty($validator)){
Matthew Weier O'Phinney Owner

This should likely check the validator type; otherwise, it's possible that the next line would call an undefined method if the validator type is not what is expected.

Tim Lieberman
timdev added a note

Not possible, since if $this->validator is anything other than InArray or Explode, $validator will be empty. Or am I missing something?

Tim Lieberman
timdev added a note

Oh, no, you're right. You can set Explode's composed validator to anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Matthew Weier O'Phinney weierophinney was assigned
Matthew Weier O'Phinney weierophinney referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#2438] CS fixes
- Trailing whitespace
be4f91d
Matthew Weier O'Phinney weierophinney referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney Merge branch 'hotfix/2438' into develop
Forward port #2438
b243c1c
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#2438] CS fixes
- Trailing whitespace
b92fb11
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney Merge branch 'hotfix/2438'
Close #2438
4ad0ce7
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney Merge branch 'hotfix/2438' into develop
Forward port #2438
626f74f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 27, 2012
  1. Improved previous fix for ZF2-558. Previous fix made implicit assumpt…

    Tim Lieberman authored
    …ions that could cause issues in subclasses of Element\[Select|MultiCheckbox]
Commits on Oct 1, 2012
  1. Improved previous fix for ZF2-558. Previous fix made implicit assumpt…

    Tim Lieberman authored
    …ions that could cause issues in subclasses of Element\[Select|MultiCheckbox]
  2. Matthew Weier O'Phinney
  3. Merge branch 'hotfix/ZF2-558-improved' of github.com:timdev/zf2 into …

    Tim Lieberman authored
    …hotfix/ZF2-558-improved
  4. Removed redundant check and fixed comment per mwop

    Tim Lieberman authored
  5. Improved previous fix for ZF2-558. Previous fix made implicit assumpt…

    Tim Lieberman authored
    …ions that could cause issues in subclasses of Element\[Select|MultiCheckbox]
  6. Removed redundant check and fixed comment per mwop

    Tim Lieberman authored
This page is out of date. Refresh to see the latest.
4 library/Zend/Form/Element/MultiCheckbox.php
View
@@ -63,8 +63,8 @@ public function setValueOptions(array $options)
{
$this->valueOptions = $options;
- // Update InArray validator haystack
- if (!is_null($this->validator)) {
+ // Update Explode validator haystack
+ if ($this->validator instanceof ExplodeValidator) {
$validator = $this->validator->getValidator();
$validator->setHaystack($this->getValueOptionsValues());
}
13 library/Zend/Form/Element/Select.php
View
@@ -83,8 +83,17 @@ public function setValueOptions(array $options)
// Update InArrayValidator validator haystack
if (!is_null($this->validator)) {
- $validator = $this->validator instanceof InArrayValidator ? $this->validator : $this->validator->getValidator();
- $validator->setHaystack($this->getValueOptionsValues());
+ if ($this->validator instanceof InArrayValidator){
+ $validator = $this->validator;
+ }
+ if ($this->validator instanceof ExplodeValidator
+ && $this->validator->getValidator() instanceof InArrayValidator
+ ){
+ $validator = $this->validator->getValidator();
+ }
+ if (!empty($validator)){
+ $validator->setHaystack($this->getValueOptionsValues());
+ }
}
return $this;
Something went wrong with that request. Please try again.