Wicket
  1. Wicket
  2. WICKET-5047

Wicket Ajax: Inline script header contribution issue

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.5.0
    • Fix Version/s: 6.7.0
    • Component/s: wicket
    • Labels:
      None

      Description

      wicket-ajax-jquery.js#processScript adds an inline script using Wicket.Head.addJavascript, in opposition to Wicket.Head.addElement for an outline script.

      The problem is that #addJavascript modifies the script content in any case, with: content = 'try

      {'+content+'}

      catch(e)

      {Wicket.Log.error(e);}

      ';

      But a script element may contains content that is not designed to be executed nor be javascript: that's the case of a jQuery Template for instance (where the tag signature look like <script id="my-id" type="text/x-jquery-tmpl">). That's also true for other known mine/type, like text/ecmascript, text/vbscript, text/tcl. And for customized ones, at least: text/x-handlebars, text/x-kendo-template, text/x-underscore-template...

      Therefore, I think ajax wicket should add script elements using #addJavascript, only if the mime type is text/javascript or is empty (because it is the default scripting language in HTML5 and the declaration can be omitted). In the other cases, it should add the script element to the DOM "as-is", using #addElement.

      Thanks in advance & best regards,
      Sebastien.

        Activity

        Hide
        Sebastien Briquet added a comment -

        Fix tested; thanks to you, Martin!

        Show
        Sebastien Briquet added a comment - Fix tested; thanks to you, Martin!
        Hide
        Martin Grigorov added a comment -

        The improvement is applied. Thanks!

        Show
        Martin Grigorov added a comment - The improvement is applied. Thanks!
        Hide
        Sebastien Briquet added a comment -

        Attached wicket-ajax-jquery.js patch.

        Show
        Sebastien Briquet added a comment - Attached wicket-ajax-jquery.js patch.
        Hide
        Sebastien Briquet added a comment -

        Hi Martin,

        Thanks for this.
        I tested your fix. unfortunately, it does not work as expected.

        I saw you extracted the "try... catch..." statement out from #addJavascript method, and test in #processScript (the caller of #addJavascript) whether the mime-type is text/javascript or is empty. In that case you embed the content with the "try... catch..." statement. But, by calling #addJavascript, the mime-type is reset to text/javascript, that is causing an error to the browser's console (you know, it is because the content block is not embedded by a "try... catch..." statement... )

        I think you have good reasons to prefer calling #addJavascript rather than #addElement, so I tried a fix by modifying the #addJavascript signature, adding the type.
        I did not saw if #addJavascript is called from outside wicket-ajax-jquery.js (except from test/js/head.js).
        Here is what I can propose:

        /*** addJavascript ***/

        addJavascript: function (content, id, fakeSrc, type) {
        var script = Wicket.Head.createElement("script");
        if (id)

        { script.id = id; }

        //WICKET-5047:
        //embeds the content with a try...catch... block iif the content is javascript
        //content is considered javascript if mime-type is empty (html5's default) or is 'text/javascript'
        if (!type || type.toLowerCase() === "text/javascript") {
        type = "text/javascript";
        content = 'try

        {'+content+'}catch(e){Wicket.Log.error(e);}';
        }

        script.setAttribute("src_", fakeSrc);
        script.setAttribute("type", type);

        // set the javascript as element content
        if (null === script.canHaveChildren || script.canHaveChildren) { var textNode = document.createTextNode(content); script.appendChild(textNode); } else { script.text = content; }
        Wicket.Head.addElement(script);
        }


        /*** addJavascripts ***/
        addJavascripts: function (element, contentFilter) {
        function add(element) {
        var src = element.getAttribute("src");
        var type = element.getAttribute("type");

        /* skipped lines for readability */

        /* TODO: to be removed
        if (!type || type.toLowerCase() === "text/javascript") {
        content = 'try{'+content+'}

        catch(e)

        {Wicket.Log.error(e);}';
        }
        */

        Wicket.Head.addJavascript(content, element.id, "", type);
        }
        }

        /* skipped lines for readability */


        /*** processScript ***/
        processScript: function (context, node) {
        context.steps.push(function (notify) {

        /* skipped lines for readability */

        var id = node.getAttribute("id");
        var type = node.getAttribute("type");

        /* TODO: to be removed
        if (!type || type.toLowerCase() === "text/javascript") {
        content = 'try{'+content+'}catch(e){Wicket.Log.error(e);}

        ';
        }
        */

        if (typeof(id) === "string" && id.length > 0)

        { // add javascript to document head Wicket.Head.addJavascript(text, id, "", type); }

        /* skipped lines for readability */

        Best regards,
        Sebastien.

        Show
        Sebastien Briquet added a comment - Hi Martin, Thanks for this. I tested your fix. unfortunately, it does not work as expected. I saw you extracted the "try... catch..." statement out from #addJavascript method, and test in #processScript (the caller of #addJavascript) whether the mime-type is text/javascript or is empty. In that case you embed the content with the "try... catch..." statement. But, by calling #addJavascript, the mime-type is reset to text/javascript, that is causing an error to the browser's console (you know, it is because the content block is not embedded by a "try... catch..." statement... ) I think you have good reasons to prefer calling #addJavascript rather than #addElement, so I tried a fix by modifying the #addJavascript signature, adding the type. I did not saw if #addJavascript is called from outside wicket-ajax-jquery.js (except from test/js/head.js). Here is what I can propose: /*** addJavascript ***/ addJavascript: function (content, id, fakeSrc, type) { var script = Wicket.Head.createElement("script"); if (id) { script.id = id; } // WICKET-5047 : //embeds the content with a try...catch... block iif the content is javascript //content is considered javascript if mime-type is empty (html5's default) or is 'text/javascript' if (!type || type.toLowerCase() === "text/javascript") { type = "text/javascript"; content = 'try {'+content+'}catch(e){Wicket.Log.error(e);}'; } script.setAttribute("src_", fakeSrc); script.setAttribute("type", type); // set the javascript as element content if (null === script.canHaveChildren || script.canHaveChildren) { var textNode = document.createTextNode(content); script.appendChild(textNode); } else { script.text = content; } Wicket.Head.addElement(script); } /*** addJavascripts ***/ addJavascripts: function (element, contentFilter) { function add(element) { var src = element.getAttribute("src"); var type = element.getAttribute("type"); /* skipped lines for readability */ /* TODO: to be removed if (!type || type.toLowerCase() === "text/javascript") { content = 'try{'+content+'} catch(e) {Wicket.Log.error(e);}'; } */ Wicket.Head.addJavascript(content, element.id, "", type); } } /* skipped lines for readability */ /*** processScript ***/ processScript: function (context, node) { context.steps.push(function (notify) { /* skipped lines for readability */ var id = node.getAttribute("id"); var type = node.getAttribute("type"); /* TODO: to be removed if (!type || type.toLowerCase() === "text/javascript") { content = 'try{'+content+'}catch(e){Wicket.Log.error(e);} '; } */ if (typeof(id) === "string" && id.length > 0) { // add javascript to document head Wicket.Head.addJavascript(text, id, "", type); } /* skipped lines for readability */ Best regards, Sebastien.
        Hide
        Martin Grigorov added a comment -

        The logic is improved.
        Please test it.

        Show
        Martin Grigorov added a comment - The logic is improved. Please test it.

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Sebastien Briquet
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development