Issue

ZF-4274: Zend_Dojo_Form Checkbox element generates incorrect HTML

Issue Type: Patch Created: 2008-09-12T21:50:41.000+0000 Last Updated: 2009-01-15T00:24:06.000+0000 Status: Closed Fix version(s): Reporter: Andrew Yager (yogel) Assignee: Bart McLeod (mcleod@spaceweb.nl) Tags: - Zend_Dojo

Related issues: Attachments: - DijitElement-ZF-4274-3.patch

Description

Zend_Dojo_Form checkbox generates incorrect HTML. Assuming a working Zend_Dojo environment, the following code produces the error:

<pre class="highlight">
class TestController extends Zend_Controller_Action {


    function indexAction () {
        
        $this->view->addHelperPath('Zend/Dojo/View/Helper/', 'Zend_Dojo_View_Helper');
        $form = new Zend_Dojo_Form();
        $form->addElement(
            'CheckBox', 
            'checkboxValue', 
            array(
                'label'        => 'Label',
                'checkedValue' => 'checkedValue',
            'uncheckedValue' => 'notCheckedValue',
            )
        );

        $form->addDecorators(array('FormElements', 'Form'));
        $this->view->form = $form;
    }
}

This produces the following HTML:

<pre class="highlight">

The correct HTML should be:

<pre class="highlight">

Tested against SVN Trunk r 11380.

Comments

Posted by Andrew Yager (yogel) on 2008-09-12T22:12:26.000+0000

This is the wrong fix, but it is a fix. The problem is that the $checkedOptions parameter is not set when passed to the Helper.

<pre class="highlight">
--- Zend/Dojo/View/Helper/CheckBox.php  (revision 11380)
+++ Zend/Dojo/View/Helper/CheckBox.php  (working copy)

 /** Zend_Dojo_View_Helper_Dijit */
@@ -72,7 +72,11 @@
         } elseif (isset($attribs['checked'])) {
             $checked = false;
         }
+
+   $checkedOptions = $attribs['options'];
+
         $checkboxInfo = Zend_View_Helper_FormCheckbox::determineCheckboxInfo($value, $checked, $checkedOptions);
         $attribs['checked'] = $checkboxInfo['checked'];
         if (!array_key_exists('id', $attribs)) {
             $attribs['id'] = $id;

Posted by Benjamin Jeanjean (apsy) on 2008-10-01T08:34:46.000+0000

This component is unusable in it's actual state... It don't work at all !

Posted by Bernd Matzner (bmatzner) on 2008-11-04T05:35:05.000+0000

In addition, the view helper adds ``` even if the form element value is set to the unChecked value. I assume this is so because the value of the hidden field differs from the unchecked value, because of which it is incorrectly assumed that the field should thus be checked.

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-06T14:20:54.000+0000

The problem was the options set in FormCheckbox.php were left unused.

We merge with existing options, because options may already be in use by multioptions.

This is a risk, so we must further test if this wil work with a checkbox group that has multioptions.

This code is added in DijitElement::render():

<pre class="highlight">
        if(array_key_exists('options', $attribs)){
            if(is_array($options)){
                $options = array_merge($options, $attribs['options']);
            }else{
                $options = $attribs['options'];
            }
        }

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-06T14:51:26.000+0000

trying to overwrite wrong patch file (is whole file instead of patch)

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-06T14:53:42.000+0000

Patch for tests/Zend/Dojo/Form/Element/CheckboxTest.php

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-06T14:56:08.000+0000

Do not use the last patch in the list (the oldest). It is the whole file (sorry). It's size is 6kb, you should use the 2kb version that is listed one higher up.

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-06T14:57:49.000+0000

Please review. I think it is quite allright, but you should take a look at how checkboxgroups behave in Dojo after patching. I should write a test for that myself, but I need to get some sleep.

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-07T03:32:15.000+0000

Found that the test patch and the dijit patch are the same: they both are the testpatch. So I try once again and upload the decorator/DijitElement.php patch.

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-18T05:23:47.000+0000

better batch, leaves multioptions intact instead of doubling them by using array_merge

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-18T06:11:32.000+0000

Better patch (number 4) deletes the lines where getMultiOptions() is called internally. This line is no longer necessary and existing tests still pass.

Also a manual test proved that these lines were no longer needed.

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-18T06:15:36.000+0000

Committed to svn after contacting Matthew about this and resolving the conflict with multioptions.

Posted by Bart McLeod (mcleod@spaceweb.nl) on 2008-11-18T06:18:09.000+0000

affects version 1.7 and prior.

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2019 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts