ZF-8011: addJavascript() uses preg_replace when trim would suffice

Issue Type: Improvement Created: 2009-10-03T12:30:45.000+0000 Last Updated: 2009-12-16T15:16:13.000+0000 Status: Resolved Fix version(s): - 1.9.7 (11/Jan/10)

Reporter: Nicholas Calugar (njcalugar) Assignee: Benjamin Eberlei (beberlei) Tags: - ZendX_JQuery

Related issues: - ZF-8560



In ZendX/JQuery/View/Helper/JQuery/Container.php - addJavascript($js) function, the first line is:

$js = preg_replace('/^\s_(._?)\s*$/s', '$1', $js);

I believe this is simply trimming whitespace from either end. trim function would be more efficient.


Posted by Mark Robinson (mrobinson_uk) on 2009-11-02T00:09:54.000+0000

If $js is a very large string then addJavascript($js) fails to add $js as expected.

In my case (checking with preg_last_error) I had reached/exceeded the backtrack limit (PREG_BACKTRACK_LIMIT_ERROR).

Using trim() may be more efficient and would also remove this limit.

Posted by Martin Minka (k2s) on 2009-12-14T11:55:39.000+0000

The preg_replace does not work with large string in deed. This is critical bug, please fix it.

My suggestion is to remove lines:

<pre class="highlight">
        $js = preg_replace('/^\s*(.*?)\s*$/s', '$1', $js);
        if (!in_array(substr($js, -1), array(';', '}'))) {
            $js .= ';';

Reasons: * Why should PHP class fix JavaScript code which a programmer wrote ? He could make much more mistakes then to forget ; in last line or to enter too many spaces. * This processing is taking unnecessary CPU on each request. * It is confusing to the JavaScript programmer that his code got changed.

If there is a plan to include some more sophisticated JavaScript minifier (I don't see a practical usage until it will not support caching) it should be optional.

Thank you for your contributed work to ZF.

Posted by Benjamin Eberlei (beberlei) on 2009-12-14T15:23:06.000+0000

On my critical list for bughunt day this week.

Posted by Benjamin Eberlei (beberlei) on 2009-12-16T14:04:32.000+0000

Fixed and merged in 1.9 release branch.

Have you found an issue?

See the Overview section for more details.


© 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.