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

I updated my bugfix code. => FIXED CODE

Changing to comply with new IT coventions for components.

On the left side the fix and on the right the current code in HeadLink.php

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.

This as been fixed with r13143