Issues

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

<pre class="highlight">
$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.

<pre class="highlight">
$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:

<pre class="highlight">
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:

<pre class="highlight">
1) Zend_View_Helper_HeadScriptTest::testSetFileStillOverwritesExistingFilesWhenItsADuplicate
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 
+

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

Fix:

<pre class="highlight">
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.

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 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