Zend Framework

DtDdWrapper is not xhtml valid

Details

  • Type: Patch Patch
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.5.1
  • Fix Version/s: 1.5.2, 1.10.5
  • Component/s: Zend_Form
  • Labels:
    None
  • Fix Version Priority:
    Must Have

Description

The empty definition term which is placed using Zend_Form when no label is specified (for example a submit button) is not XHTML valid. Therefore I suggest that, if the doctype is set to XHTML, the wrapper either doesn't place the empty definition term <dt></dt>, or places a nonbraking space inside it <dt> </dt>.

Issue Links

This issue is related to:
ZF-3339 DtDdWrapper is not xhtml valid for radio buttons Major Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Wil Sinclair added a comment -

Please evaluate and categorize as necessary.

Show
Wil Sinclair added a comment - Please evaluate and categorize as necessary.
Hide
Permalink
Matthew Weier O'Phinney added a comment -

Scheduling for next mini release.

Show
Matthew Weier O'Phinney added a comment - Scheduling for next mini release.
Hide
Permalink
Matthew Weier O'Phinney added a comment -

fixed in trunk and release-1.5 branch as of r9310.

Show
Matthew Weier O'Phinney added a comment - fixed in trunk and release-1.5 branch as of r9310.
Hide
Permalink
Robin Skoglund added a comment -

The situation described is not invalid XHTML, and the non-breaking space should be removed again.

Show
Robin Skoglund added a comment - The situation described is not invalid XHTML, and the non-breaking space should be removed again.
Hide
Permalink
Dimitri van Hees added a comment -

uhm... yes it is invalid..?

Show
Dimitri van Hees added a comment - uhm... yes it is invalid..?
Hide
Permalink
Robin Skoglund added a comment -

Where does it say it's valid? I am unable to find a source that backs up your statement, and according to the W3C validator the document is valid. You shouldn't just say "umh, yes it is invalid?" without backing it up.

http://www.w3.org/TR/xhtml1/
http://www.w3.org/TR/1999/REC-html401-19991224/struct/lists.html#edef-DT
http://www.w3schools.com/tags/tag_dt.asp

Show
Robin Skoglund added a comment - Where does it say it's valid? I am unable to find a source that backs up your statement, and according to the W3C validator the document is valid. You shouldn't just say "umh, yes it is invalid?" without backing it up. http://www.w3.org/TR/xhtml1/ http://www.w3.org/TR/1999/REC-html401-19991224/struct/lists.html#edef-DT http://www.w3schools.com/tags/tag_dt.asp
Hide
Permalink
Robin Skoglund added a comment -

Also, the reason I'm complaining is this:
With an empty <dt></dt>, the element is not rendered, and the <dd>...</dd> below will position itself where the dt element would be, whereas with   the dt element renders and takes up visual space on the page. This means that by default you get a notable gap between the last form element and the submit button, which looks odd (it looks like the form is missing something), and if you care about visual design you have to turn to various hacks to "remove the gap". Far from ideal.

Show
Robin Skoglund added a comment - Also, the reason I'm complaining is this: With an empty <dt></dt>, the element is not rendered, and the <dd>...</dd> below will position itself where the dt element would be, whereas with   the dt element renders and takes up visual space on the page. This means that by default you get a notable gap between the last form element and the submit button, which looks odd (it looks like the form is missing something), and if you care about visual design you have to turn to various hacks to "remove the gap". Far from ideal.
Hide
Permalink
Matthew Weier O'Phinney added a comment -

People reported on-list that it was invalid markup, and provided the warnings that the w3c validator raised. However, just to verify, I tried myself with the following snippet:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<title>Foo</title>
</head>
<body>
<dl>
<dt></dt>
<dd>foobar!</dd>
</dl>
</body>
</html>

and this validates. So, I think the appropriate thing to do here is to provide a switch to the DtDdWrapper, asking to either insert the no-break space or not, and have it default to off.

Show
Matthew Weier O'Phinney added a comment - People reported on-list that it was invalid markup, and provided the warnings that the w3c validator raised. However, just to verify, I tried myself with the following snippet: 
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<title>Foo</title>
</head>
<body>
<dl>
<dt></dt>
<dd>foobar!</dd>
</dl>
</body>
</html>
and this validates. So, I think the appropriate thing to do here is to provide a switch to the DtDdWrapper, asking to either insert the no-break space or not, and have it default to off.
Hide
Permalink
Robin Skoglund added a comment -

The attached file passes W3C validation.

Tip: Copy-paste the file contents into this: http://validator.w3.org/#validate_by_input+with_options

Show
Robin Skoglund added a comment - The attached file passes W3C validation. Tip: Copy-paste the file contents into this: http://validator.w3.org/#validate_by_input+with_options
Hide
Permalink
Robin Skoglund added a comment -

Sounds good, Matthew

Show
Robin Skoglund added a comment - Sounds good, Matthew
Hide
Permalink
Darren Lucas added a comment -

The empty definition term tag issue is probably caused from using HTML Tidy to validate, e.g running the above snippet through Tidy will show the following as an error:

 trimming empty <dt>
{/noformat}

Tested using: [http://validator.aborla.net]

In this regard Tidy is a bit over keen, it shouldn't be throwing errors for things that are perfectly valid.  In fact HTML Tidy shouldn't really be used as a validator, unfortunately it is used by certain Firefox HTML validator plugins for this purpose.


Matthews suggestion of providing a switch to the DtDdWrapper asking to insert a non-breaking space is a good solution.  As well as a non-breaking space could an option to put empty comment tags be added, e.g:

<dt><!-- --></dt>

{/noformat}

This works around the HTML Tidy issue without adding whitespace into the rendered HTML page, the non-breaking space will increase or decrease in size depending on the font size.

Show
Darren Lucas added a comment - The empty definition term tag issue is probably caused from using HTML Tidy to validate, e.g running the above snippet through Tidy will show the following as an error: 
 trimming empty <dt>
{/noformat}

Tested using: [http://validator.aborla.net]

In this regard Tidy is a bit over keen, it shouldn't be throwing errors for things that are perfectly valid.  In fact HTML Tidy shouldn't really be used as a validator, unfortunately it is used by certain Firefox HTML validator plugins for this purpose.


Matthews suggestion of providing a switch to the DtDdWrapper asking to insert a non-breaking space is a good solution.  As well as a non-breaking space could an option to put empty comment tags be added, e.g:
<dt><!-- --></dt> {/noformat} This works around the HTML Tidy issue without adding whitespace into the rendered HTML page, the non-breaking space will increase or decrease in size depending on the font size.
Hide
Permalink
Dolf Schimmel (Freeaqingme) added a comment -

The issue clearly wasn't fixed with 1.5.2 (else the issue would have been closed?) so could someone please give it a new fix version, or postpone it?

Show
Dolf Schimmel (Freeaqingme) added a comment - The issue clearly wasn't fixed with 1.5.2 (else the issue would have been closed?) so could someone please give it a new fix version, or postpone it?
Hide
Permalink
Matthew Weier O'Phinney added a comment -

Setting fix version to unknown. The 1.5.2 fix version was regarding the original fix (adding the no-break space).

Show
Matthew Weier O'Phinney added a comment - Setting fix version to unknown. The 1.5.2 fix version was regarding the original fix (adding the no-break space).
Hide
Permalink
Christian Albrecht added a comment -

made a patch for converting &nbsp; to &#160; after that i detected this Issue,
so i merged the other into this one. The patch is easy converted to take <!-- -->
instead.

Show
Christian Albrecht added a comment - made a patch for converting &nbsp; to &#160; after that i detected this Issue, so i merged the other into this one. The patch is easy converted to take <!-- --> instead.
Hide
Permalink
Christian Albrecht added a comment -

Because i think the argument weights, that adding whitespace into rendered html is bad,
here the updated patch from SubTask

Index: tests/Zend/Form/SubFormTest.php
===================================================================
--- tests/Zend/Form/SubFormTest.php     (Revision 21740)
+++ tests/Zend/Form/SubFormTest.php     (Arbeitskopie)
@@ -130,7 +130,7 @@
         $form->addSubForm($subForm, 'foobar')
              ->setView(new Zend_View);
         $html = $form->render();
-        $this->assertContains('<dt>&nbsp;</dt>', $html);
+        $this->assertContains('<dt><!-- --></dt>', $html);
     }
 }
 
Index: tests/Zend/Form/DisplayGroupTest.php
===================================================================
--- tests/Zend/Form/DisplayGroupTest.php        (Revision 21740)
+++ tests/Zend/Form/DisplayGroupTest.php        (Arbeitskopie)
@@ -401,7 +401,7 @@
 
         $this->group->addElements(array($foo, $bar));
         $html = $this->group->render($this->getView());
-        $this->assertRegexp('#^<dt[^>]*>&nbsp;</dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
+        $this->assertRegexp('#^<dt[^>]*><!-- --></dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
         $this->assertContains('<input', $html, $html);
         $this->assertContains('"foo"', $html);
         $this->assertContains('"bar"', $html);
@@ -415,7 +415,7 @@
         $this->group->addElements(array($foo, $bar))
                     ->setView($this->getView());
         $html = $this->group->__toString();
-        $this->assertRegexp('#^<dt[^>]*>&nbsp;</dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
+        $this->assertRegexp('#^<dt[^>]*><!-- --></dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
         $this->assertContains('<input', $html);
         $this->assertContains('"foo"', $html);
         $this->assertContains('"bar"', $html);
Index: tests/Zend/Form/Element/RadioTest.php
===================================================================
--- tests/Zend/Form/Element/RadioTest.php       (Revision 21740)
+++ tests/Zend/Form/Element/RadioTest.php       (Arbeitskopie)
@@ -168,7 +168,7 @@
                 'test' => 'Test',
             ));
         $html = $this->element->render($this->getView());
-        $this->assertRegexp('#<dt[^>]*>&nbsp;</dt>.*?<dd#s', $html, $html);
+        $this->assertRegexp('#<dt[^>]*><!-- --></dt>.*?<dd#s', $html, $html);
     }
 
     /**
Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php        (Revision 21740)
+++ tests/Zend/Form/FormTest.php        (Arbeitskopie)
@@ -3757,7 +3757,7 @@
 
         $html = $form->render();
 
-        $this->assertContains('<dt id="foo-label">&nbsp;</dt>', $html);
+        $this->assertContains('<dt id="foo-label"><!-- --></dt>', $html);
         $this->assertContains('<dd id="foo-element">', $html);
     }
 
@@ -3773,7 +3773,7 @@
 
         $html = $form->render();
 
-        $this->assertContains('<dt id="testform-label">&nbsp;</dt>', $html);
+        $this->assertContains('<dt id="testform-label"><!-- --></dt>', $html);
         $this->assertContains('<dd id="testform-element">', $html);
     }
 
Index: library/Zend/Form/Decorator/Label.php
===================================================================
--- library/Zend/Form/Decorator/Label.php       (Revision 21740)
+++ library/Zend/Form/Decorator/Label.php       (Arbeitskopie)
@@ -310,7 +310,7 @@
             $options['class'] = $class;
             $label = $view->formLabel($element->getFullyQualifiedName(), trim($label), $options);
         } else {
-            $label = '&nbsp;';
+            $label = '<!-- -->';
         }
 
         if (null !== $tag) {
Index: library/Zend/Form/Decorator/DtDdWrapper.php
===================================================================
--- library/Zend/Form/Decorator/DtDdWrapper.php (Revision 21740)
+++ library/Zend/Form/Decorator/DtDdWrapper.php (Arbeitskopie)
@@ -57,7 +57,7 @@
     {
         $elementName = $this->getElement()->getName();
 
-        return '<dt id="' . $elementName . '-label">&nbsp;</dt>' .
+        return '<dt id="' . $elementName . '-label"><!-- --></dt>' .
                '<dd id="' . $elementName . '-element">' . $content . '</dd>';
     }
 }
Show
Christian Albrecht added a comment - Because i think the argument weights, that adding whitespace into rendered html is bad, here the updated patch from SubTask 
Index: tests/Zend/Form/SubFormTest.php
===================================================================
--- tests/Zend/Form/SubFormTest.php     (Revision 21740)
+++ tests/Zend/Form/SubFormTest.php     (Arbeitskopie)
@@ -130,7 +130,7 @@
         $form->addSubForm($subForm, 'foobar')
              ->setView(new Zend_View);
         $html = $form->render();
-        $this->assertContains('<dt>&nbsp;</dt>', $html);
+        $this->assertContains('<dt><!-- --></dt>', $html);
     }
 }
 
Index: tests/Zend/Form/DisplayGroupTest.php
===================================================================
--- tests/Zend/Form/DisplayGroupTest.php        (Revision 21740)
+++ tests/Zend/Form/DisplayGroupTest.php        (Arbeitskopie)
@@ -401,7 +401,7 @@
 
         $this->group->addElements(array($foo, $bar));
         $html = $this->group->render($this->getView());
-        $this->assertRegexp('#^<dt[^>]*>&nbsp;</dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
+        $this->assertRegexp('#^<dt[^>]*><!-- --></dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
         $this->assertContains('<input', $html, $html);
         $this->assertContains('"foo"', $html);
         $this->assertContains('"bar"', $html);
@@ -415,7 +415,7 @@
         $this->group->addElements(array($foo, $bar))
                     ->setView($this->getView());
         $html = $this->group->__toString();
-        $this->assertRegexp('#^<dt[^>]*>&nbsp;</dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
+        $this->assertRegexp('#^<dt[^>]*><!-- --></dt><dd[^>]*><fieldset.*?</fieldset></dd>$#s', $html, $html);
         $this->assertContains('<input', $html);
         $this->assertContains('"foo"', $html);
         $this->assertContains('"bar"', $html);
Index: tests/Zend/Form/Element/RadioTest.php
===================================================================
--- tests/Zend/Form/Element/RadioTest.php       (Revision 21740)
+++ tests/Zend/Form/Element/RadioTest.php       (Arbeitskopie)
@@ -168,7 +168,7 @@
                 'test' => 'Test',
             ));
         $html = $this->element->render($this->getView());
-        $this->assertRegexp('#<dt[^>]*>&nbsp;</dt>.*?<dd#s', $html, $html);
+        $this->assertRegexp('#<dt[^>]*><!-- --></dt>.*?<dd#s', $html, $html);
     }
 
     /**
Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php        (Revision 21740)
+++ tests/Zend/Form/FormTest.php        (Arbeitskopie)
@@ -3757,7 +3757,7 @@
 
         $html = $form->render();
 
-        $this->assertContains('<dt id="foo-label">&nbsp;</dt>', $html);
+        $this->assertContains('<dt id="foo-label"><!-- --></dt>', $html);
         $this->assertContains('<dd id="foo-element">', $html);
     }
 
@@ -3773,7 +3773,7 @@
 
         $html = $form->render();
 
-        $this->assertContains('<dt id="testform-label">&nbsp;</dt>', $html);
+        $this->assertContains('<dt id="testform-label"><!-- --></dt>', $html);
         $this->assertContains('<dd id="testform-element">', $html);
     }
 
Index: library/Zend/Form/Decorator/Label.php
===================================================================
--- library/Zend/Form/Decorator/Label.php       (Revision 21740)
+++ library/Zend/Form/Decorator/Label.php       (Arbeitskopie)
@@ -310,7 +310,7 @@
             $options['class'] = $class;
             $label = $view->formLabel($element->getFullyQualifiedName(), trim($label), $options);
         } else {
-            $label = '&nbsp;';
+            $label = '<!-- -->';
         }
 
         if (null !== $tag) {
Index: library/Zend/Form/Decorator/DtDdWrapper.php
===================================================================
--- library/Zend/Form/Decorator/DtDdWrapper.php (Revision 21740)
+++ library/Zend/Form/Decorator/DtDdWrapper.php (Arbeitskopie)
@@ -57,7 +57,7 @@
     {
         $elementName = $this->getElement()->getName();
 
-        return '<dt id="' . $elementName . '-label">&nbsp;</dt>' .
+        return '<dt id="' . $elementName . '-label"><!-- --></dt>' .
                '<dd id="' . $elementName . '-element">' . $content . '</dd>';
     }
 }
Hide
Permalink
Christian Albrecht added a comment -

Rescheduling for next Minor Release as the workarounds for the whitespace must be adapted by Developers.

Show
Christian Albrecht added a comment - Rescheduling for next Minor Release as the workarounds for the whitespace must be adapted by Developers.
Hide
Permalink
Christian Albrecht added a comment - - edited

Fixed in trunk r22128 and merged into 1.10 release branch.

DtDdWrapper retrieves the filler for the label now from option 'dtLabel',
if this is not set defaults to '&#160;' //edit: stupid whitespace

Zend_From_Decorator_DtDdWrapper
public function render($content)
    {
        $elementName = $this->getElement()->getName();
        
        $dtLabel = $this->getOption('dtLabel');
        if( null === $dtLabel ) {
            $dtLabel = '&#160;';
        }

        return '<dt id="' . $elementName . '-label">' . $dtLabel . '</dt>' .
               '<dd id="' . $elementName . '-element">' . $content . '</dd>';
    }
Show
Christian Albrecht added a comment - - edited Fixed in trunk r22128 and merged into 1.10 release branch. DtDdWrapper retrieves the filler for the label now from option 'dtLabel', if this is not set defaults to '&#160;' //edit: stupid whitespace
Zend_From_Decorator_DtDdWrapper
public function render($content)
    {
        $elementName = $this->getElement()->getName();
        
        $dtLabel = $this->getOption('dtLabel');
        if( null === $dtLabel ) {
            $dtLabel = '&#160;';
        }

        return '<dt id="' . $elementName . '-label">' . $dtLabel . '</dt>' .
               '<dd id="' . $elementName . '-element">' . $content . '</dd>';
    }

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:

Time Tracking

Estimated:
30m
Original Estimate - 30 minutes
Remaining:
30m
Remaining Estimate - 30 minutes
Logged:
Not Specified
Time Spent - Not Specified
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.