Zend Framework

addJavascript() uses preg_replace when trim would suffice

Details

  • Type: Improvement Improvement
  • Status: Resolved Resolved
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.9.7
  • Component/s: ZendX_JQuery
  • Labels:
    None

Description

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.

Issue Links

This issue is related to:
ZF-8560 remove unnecessary parameter manipulation in ZendX_JQuery_View_Helper_JQuery_Container::addJavascript() Minor Resolved

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Mark Robinson added a comment -

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.

Show
Mark Robinson added a comment - 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.
Hide
Permalink
Martin Minka added a comment -

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

My suggestion is to remove lines:

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

Show
Martin Minka added a comment - The preg_replace does not work with large string in deed. This is critical bug, please fix it. My suggestion is to remove lines: 
$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.
Hide
Permalink
Benjamin Eberlei added a comment -

On my critical list for bughunt day this week.

Show
Benjamin Eberlei added a comment - On my critical list for bughunt day this week.
Hide
Permalink
Benjamin Eberlei added a comment -

Fixed and merged in 1.9 release branch.

Show
Benjamin Eberlei added a comment - Fixed and merged in 1.9 release branch.

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.