ZF-3271: Zend_View_Helper_HeadLink - bug with appendStylesheet()
Description
hi people,
i discussed the problem with Thomas in an issue of 1.5.1 - http://framework.zend.com/issues/browse/ZF-2889, but i think the fixed code is still a bug.
Bug #1 is in the manual. It says "$conditionalStylesheet is a boolean....". That's wrong, because $conditionalStylesheet has to be a string. A boolean doesn't make sense and in PHP twice, because of its weak and dynamic typing.
Let's have a look at the code in line 281 of HeadLink.php: if (isset($attributes['conditionalStylesheet']) && (false !== $attributes['conditionalStylesheet'])) { $link = ''; }
If $attributes['conditionalStylesheet'] is a boolean like the manual says (only TRUE is accepted, so why boolean?! And then, why do not write "if (true === $attributes['conditionalStylesheet'])" ;) ) then it won't be casted (see Bug #2) to string "true" but to string "1". Result will be: <!--[if 1]>
This is senseless and wrong. See valid values here: http://msdn.microsoft.com/en-us/library/…
So a string "true" will be correct instead of boolean true. A boolean false or true generates invalid output (see Bug #2). Only strings should be accepted, like in the link above.
Bug #2 is about the code in line 346 of HeadLink.php. I think the following changes will fix the bug:
CURRENT CODE: if (false !== $conditionalStylesheet){ $conditionalStylesheet = (string) $conditionalStylesheet; }
FIXED CODE if (!empty($conditionalStylesheet) && is_string($conditionalStylesheet)) { $conditionalStylesheet = (string) $conditionalStylesheet; } else { $conditionalStylesheet = null; }
Now, outputs like [if ] or [if 1] won't be possible anymore.
And following useful code in an application will be possible now:
foreach ($styleDefs as $key => $array) {
$this->headLink()->appendStylesheet($array['href'], $array['media'], $array['conditionalStylesheet']);
}
Maybe $styleDefs comes from a config file and 'conditionalStylesheet' is an empty string there. Before the fix the code would have produced senseless output like [if ]. Now, $conditionalStylesheet is NOT SET when the code at line 281 (see Bug #1) is executed and it's all running fine :)
Sure, in my app code i could check the arguments before giving them to appendStylesheet(), but that's not the point here. ;)
greets, David
btw: sorry for editing twice...it is 01:00 A.M. in Germany ;)
Comments
Posted by David Toniolo (david toniolo) on 2008-06-09T03:59:53.000+0000
I updated my bugfix code. => FIXED CODE
Posted by Wil Sinclair (wil) on 2008-06-15T14:03:42.000+0000
Changing to comply with new IT coventions for components.
Posted by David Toniolo (david toniolo) on 2008-08-15T14:52:37.000+0000
On the left side the fix and on the right the current code in HeadLink.php
Posted by Jon Whitcraft (sidhighwind) on 2008-10-28T19:26:50.000+0000
Can you provide a patch file from the current SVN? The code has changed and I dont see the code that you are referencing in the HeadLink.php File.
Posted by Jon Whitcraft (sidhighwind) on 2008-12-10T16:44:13.000+0000
This as been fixed with r13143