ZF-12048 - Issues - Zend Framework

ZF-12048: Zend_View_Helper_HeadScript::setFile behavior inconsistent (based on order of file addition)

Issue Type:

Bug

Created:

2012-02-09T15:46:55.000+0000

Last Updated:

2012-06-04T14:43:19.000+0000

Status:

Resolved

Fix version(s):

1.12.0 (27/Aug/12)

Reporter:

Arno Schäfer (arnoschaefer)

Assignee:

Adam Lundrigan (adamlundrigan)

Tags:

Zend_View
FixForZF1.12
zf-caretaker-adamlundrigan
zf-crteam-review

Related issues:

Attachments:

ZF-12048.patch

Description

In my understanding, the "setFile" method in the headScript helper should replace all previously added items (the doc does not say much about this). This example works as expected:

$view->headScript()->appendFile ("/_files/js/foo.js"); $view->headScript()->appendFile ("/_files/js/bar.js");

...

$view->headScript()->setFile ("/_files/js/baz.js");

In this case, the result is that foo.js and bar.js are removed and baz.js is added.

However:

$view->headScript()->setFile ("/_files/js/foo.js");

In this case, because foo.js has been appended before and there is an _isDuplicate() check first, the call is ignored.

I would regard this an error, because IMHO "set" should replace all previously added content. Suggested fix:

if (!$this->_isDuplicate($content)) {

if (!$this->_isDuplicate($content) || "set" == $action) {

The same thing happends with headLink ()->setStylesheet (), although the code is a bit different.

Comments

Posted by Ricardo Buquet (enkil2003) on 2012-02-11T00:14:30.000+0000

I cannot reproduce the error you are describing. This is my code, and output


$view->headScript()->appendFile ("/_files/js/foo.js");
$view->headScript()->appendFile ("/_files/js/bar.js");
$view->headScript()->setFile ("/_files/js/baz.js");
$view->headScript()->setFile ("/_files/js/foo.js");
        
foreach($view->headScript() as $file) {
    echo $file->attributes['src'] ."\n";
}

// output
/_files/js/foo.js

which is what I'm expecting. I have also tried this code and works like a charm.


$view->headScript()->appendFile ("/_files/js/foo.js");
$view->headScript()->appendFile ("/_files/js/bar.js");
$view->headScript()->setFile ("/_files/js/baz.js");
$view->headScript()->setFile ("/_files/js/foo.js");
$view->headScript()->appendFile ("/_files/js/bar.js");
        
foreach($view->headScript() as $file) {
    echo $file->attributes['src'] ."\n";
}

// output
/_files/js/foo.js
/_files/js/bar.js

Posted by Arno Schäfer (arnoschaefer) on 2012-02-11T08:14:35.000+0000

Sorry for being not entirely clear. Try this:

$view->headScript()->appendFile ("/_files/js/foo.js"); $view->headScript()->appendFile ("/_files/js/bar.js"); $view->headScript()->setFile ("/_files/js/foo.js");

foreach($view->headScript() as $file) { echo $file->attributes['src'] ."\n"; }

// output /_files/js/foo.js /_files/js/bar.js

I believe based on the "set" call, it should be

/_files/js/foo.js

Posted by Adam Lundrigan (adamlundrigan) on 2012-02-25T17:51:56.000+0000

Confirmed. Test:


Index: tests/Zend/View/Helper/HeadScriptTest.php
===================================================================
--- tests/Zend/View/Helper/HeadScriptTest.php   (revision 24628)
+++ tests/Zend/View/Helper/HeadScriptTest.php   (working copy)
@@ -451,6 +451,20 @@

         $this->assertEquals($expected, $test);
     }
+
+    /**
+     * @group ZF-12048
+     */
+    public function testSetFileStillOverwritesExistingFilesWhenItsADuplicate()
+    {
+        $this->helper->appendFile('foo.js');
+        $this->helper->appendFile('bar.js');
+        $this->helper->setFile('foo.js');
+
+        $expected = '';
+        $test = $this->helper->toString();
+        $this->assertEquals($expected, $test);
+    }
 }

 // Call Zend_View_Helper_HeadScriptTest::main() if this source file is executed directly.

Result:


1) Zend_View_Helper_HeadScriptTest::testSetFileStillOverwritesExistingFilesWhenItsADuplicate
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 
+

tests/Zend/View/Helper/HeadScriptTest.php:466

Fix:


Index: library/Zend/View/Helper/HeadScript.php
===================================================================
--- library/Zend/View/Helper/HeadScript.php     (revision 24628)
+++ library/Zend/View/Helper/HeadScript.php     (working copy)
@@ -245,7 +245,7 @@
                     break;
                 case 'file':
                 default:
-                    if (!$this->_isDuplicate($content)) {
+                    if (!$this->_isDuplicate($content) || $action=='set') {
                         $attrs['src'] = $content;
                         $item = $this->createData($type, $attrs);
                         if ('offsetSet' == $action) {

Posted by Arno Schäfer (arnoschaefer) on 2012-02-25T22:11:08.000+0000

Great, thanks for this.

Please note that the same problem still exists with headLink ()->setStylesheet (), so you might not want to close this bug just yet. If I get around to it, I will look into the code, maybe I can suggest a fix for this as well.

Posted by Adam Lundrigan (adamlundrigan) on 2012-06-04T14:43:19.000+0000

Fixed in trunk (1.12.0): r24878

Arno: The HeadLink problem is a distinct issue, so could you please open a new ticket for that one? Thanks.