Tapestry
  1. Tapestry
  2. TAPESTRY-788

DatePicker doesn't work in a portlet. Initialization script is run before window finishes loading.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.1
    • Component/s: Portlet
    • Labels:
      None
    • Environment:
      Tapestry 4.0-beta-13
      Jetspeed 2 portal

      Description

      When using a DatePicker in a Tapestry portlet, IE displays an error message during page loading and aborts:
      "Internet Explorer cannot open the site http://localhost. Operation aborted"

      It appears that this is caused by the initialization script of the DatePicker component being run before the window finishes loading. The script is run right before the portlet finishes rendering, which, in a portal environment, is not necessarily the end of the page.

      1. TAPESTRY-788-p1.patch
        0.2 kB
        Sean Tan
      2. TAPESTRY-788-p2.patch
        0.6 kB
        Sean Tan

        Activity

        Hide
        Raphael Jean added a comment -

        Here is a replacement for the PageRenderSupportImpl.writeInitializationScript() method.
        It uses the window.onload event handler to run the initialization script, but is nice with other portlets handlers by setting up a handler chain.

        public void writeInitializationScript(IMarkupWriter writer)
        {
        if (!any(_initializationScript))
        return;

        writer.begin("script");
        writer.attribute("language", "JavaScript");
        writer.attribute("type", "text/javascript");
        writer.printRaw("<!--\n");

        String previousOnload = getUniqueString("previousWindowOnload");

        writer.printRaw("var " + previousOnload + " = window.onload;\n");

        writer.printRaw("window.onload = function()

        {\n"); writer.printRaw("if ("); writer.printRaw(previousOnload); writer.printRaw(")\n\t"); writer.printRaw(previousOnload); writer.printRaw("();\n"); writer.printRaw(_initializationScript.toString()); writer.printRaw("}

        \n");
        writer.printRaw("\n// -->");
        writer.end();
        }

        Show
        Raphael Jean added a comment - Here is a replacement for the PageRenderSupportImpl.writeInitializationScript() method. It uses the window.onload event handler to run the initialization script, but is nice with other portlets handlers by setting up a handler chain. public void writeInitializationScript(IMarkupWriter writer) { if (!any(_initializationScript)) return; writer.begin("script"); writer.attribute("language", "JavaScript"); writer.attribute("type", "text/javascript"); writer.printRaw("<!--\n"); String previousOnload = getUniqueString("previousWindowOnload"); writer.printRaw("var " + previousOnload + " = window.onload;\n"); writer.printRaw("window.onload = function() {\n"); writer.printRaw("if ("); writer.printRaw(previousOnload); writer.printRaw(")\n\t"); writer.printRaw(previousOnload); writer.printRaw("();\n"); writer.printRaw(_initializationScript.toString()); writer.printRaw("} \n"); writer.printRaw("\n// -->"); writer.end(); }
        Hide
        Jesse Kuhnert added a comment -

        This should get resolved a little more definitively in tapestry 4.1 when most of the js related portions of tapestry are tweaked a little bit to allow for a more advanced means of handling browser based events in general.

        Show
        Jesse Kuhnert added a comment - This should get resolved a little more definitively in tapestry 4.1 when most of the js related portions of tapestry are tweaked a little bit to allow for a more advanced means of handling browser based events in general.
        Hide
        Anders Bengtsson added a comment -

        Any chance of a fix for this in a bug-fix release for 4.0?

        Show
        Anders Bengtsson added a comment - Any chance of a fix for this in a bug-fix release for 4.0?
        Hide
        Jesse Kuhnert added a comment -

        The initial reason for not wanting to apply this is that calling window.onload effectively obliterates any chance anyone else has of also capturing this event.

        I'm thinking about using this script to do it, http://dean.edwards.name/my/events.js, but am still unsure as it's such a potentially impactual change.

        Show
        Jesse Kuhnert added a comment - The initial reason for not wanting to apply this is that calling window.onload effectively obliterates any chance anyone else has of also capturing this event. I'm thinking about using this script to do it, http://dean.edwards.name/my/events.js , but am still unsure as it's such a potentially impactual change.
        Hide
        Raphael Jean added a comment -

        Note that this patch will save any existing window.onload event and call it before running the tapestry initialization script.

        Show
        Raphael Jean added a comment - Note that this patch will save any existing window.onload event and call it before running the tapestry initialization script.
        Hide
        Jesse Kuhnert added a comment -

        The window.onload is undesirable for a lot of different reasons, one in particular that would affect me directly is that in the tacos ajax world there is no window.onload event for most request. The window gets loaded once and scritps are eval'd from responses.

        Is there a known way to push this render further down the pipeline, or is this just a limitation of the portlet spec?

        Perhaps we just need to fix what the DatePicker script is doing instead? That would be much preferred. Do you by any chance have IE's script debugger installed? (it's an optional component you can install with ms office cd's ) Perhaps there is a specific line or method that causes the error in question?

        I know for ajax type stuff at least that most IE related javascript calls have to be executed off the main rendering thread. This is usually solved by doing a setTimeout(function(), 50) which causes the scrips to be executed 50 ms later.

        I can try this if you like, or is this a global problem not just specific to DatePicker?

        Show
        Jesse Kuhnert added a comment - The window.onload is undesirable for a lot of different reasons, one in particular that would affect me directly is that in the tacos ajax world there is no window.onload event for most request. The window gets loaded once and scritps are eval'd from responses. Is there a known way to push this render further down the pipeline, or is this just a limitation of the portlet spec? Perhaps we just need to fix what the DatePicker script is doing instead? That would be much preferred. Do you by any chance have IE's script debugger installed? (it's an optional component you can install with ms office cd's ) Perhaps there is a specific line or method that causes the error in question? I know for ajax type stuff at least that most IE related javascript calls have to be executed off the main rendering thread. This is usually solved by doing a setTimeout(function(), 50) which causes the scrips to be executed 50 ms later. I can try this if you like, or is this a global problem not just specific to DatePicker?
        Hide
        Raphael Jean added a comment -

        I found out that the problem happens when the Calendar main DIV is appended to the document body. This happens at the very end of the Calendar.create() function. If IE hasn't finished rendering the page at this moment, then an error occurs.
        This problem is not limited to portlets by the way. I've heard people having the same problem when including a jsp footer.

        Using a timer would probably help but will probably not eliminate the problem completely as rendering a portal page can take a long time.

        Another way to fix this specific problem with DatePicker is to delay the creation of the floating DIV until the DatePicker is opened:
        Calendar.prototype.toggle = function(element) {
        + if (this._calDiv == null)
        + create();
        if(this._showing)

        { this.hide(); }

        else

        { this.show(element); }

        }

        and remove the this.create() call in Calendar.initialize()

        This seems to work fine.

        Show
        Raphael Jean added a comment - I found out that the problem happens when the Calendar main DIV is appended to the document body. This happens at the very end of the Calendar.create() function. If IE hasn't finished rendering the page at this moment, then an error occurs. This problem is not limited to portlets by the way. I've heard people having the same problem when including a jsp footer. Using a timer would probably help but will probably not eliminate the problem completely as rendering a portal page can take a long time. Another way to fix this specific problem with DatePicker is to delay the creation of the floating DIV until the DatePicker is opened: Calendar.prototype.toggle = function(element) { + if (this._calDiv == null) + create(); if(this._showing) { this.hide(); } else { this.show(element); } } and remove the this.create() call in Calendar.initialize() This seems to work fine.
        Hide
        Jesse Kuhnert added a comment -

        That sounds like a great idea. I will do it soon. Thank you.

        Show
        Jesse Kuhnert added a comment - That sounds like a great idea. I will do it soon. Thank you.
        Hide
        Sean Tan added a comment -

        In response to the solution posted by Raphael Jean, it works fine for me except that I need to change the call to create() to this.create().

        Meaning,
        Calendar.prototype.toggle = function(element) {
        + if (this._calDiv == null)
        + this.create();
        if(this._showing)

        { this.hide(); }

        else

        { this.show(element); }

        }

        and remove the this.create() call in Calendar.initialize()

        Also, just to note that this patch is not included in the newly released Tapestry version 4.0.1.

        Show
        Sean Tan added a comment - In response to the solution posted by Raphael Jean, it works fine for me except that I need to change the call to create() to this.create(). Meaning, Calendar.prototype.toggle = function(element) { + if (this._calDiv == null) + this.create(); if(this._showing) { this.hide(); } else { this.show(element); } } and remove the this.create() call in Calendar.initialize() Also, just to note that this patch is not included in the newly released Tapestry version 4.0.1.
        Hide
        Jesse Kuhnert added a comment -

        Hmm..This must have slipped through the radar somehow. Can someone attach an actual patch for me?

        Show
        Jesse Kuhnert added a comment - Hmm..This must have slipped through the radar somehow. Can someone attach an actual patch for me?
        Hide
        Sean Tan added a comment -

        I'm not sure whether the patch is done properly since it is my first time. =b

        Anyway, here you go.

        Show
        Sean Tan added a comment - I'm not sure whether the patch is done properly since it is my first time. =b Anyway, here you go.
        Hide
        Sean Tan added a comment -

        Just realized that the http://svn.apache.org/repos/asf/jakarta/tapestry/tags/4.0.1/framework/src/java/org/apache/tapestry/form/DatePicker.js is marked as
        svn:mime-type application/octet-stream
        which should not be.

        I wonder whether this is affecting the patch which i uploaded because I can't even apply my own patch using tortoisesvn.

        I guess someone needs to remove the svn:mime-type application/octet-stream before I can do and upload a proper patch?
        And may I know which branch I should create the patch against. Thanks.

        Show
        Sean Tan added a comment - Just realized that the http://svn.apache.org/repos/asf/jakarta/tapestry/tags/4.0.1/framework/src/java/org/apache/tapestry/form/DatePicker.js is marked as svn:mime-type application/octet-stream which should not be. I wonder whether this is affecting the patch which i uploaded because I can't even apply my own patch using tortoisesvn. I guess someone needs to remove the svn:mime-type application/octet-stream before I can do and upload a proper patch? And may I know which branch I should create the patch against. Thanks.
        Hide
        Jesse Kuhnert added a comment -

        I've removed the octet-stream mime type. Any chance you could create a new patch for me? (won't let me apply the old one now )

        Show
        Jesse Kuhnert added a comment - I've removed the octet-stream mime type. Any chance you could create a new patch for me? (won't let me apply the old one now )
        Show
        Sean Tan added a comment - Here's the patch I have created against http://svn.apache.org/repos/asf/jakarta/tapestry/branches/4.0/framework/src/java/org/apache/tapestry/form/DatePicker.js
        Hide
        Jesse Kuhnert added a comment -

        Oops.. Didn't even notice I had fixed this one. Should work now.

        Show
        Jesse Kuhnert added a comment - Oops.. Didn't even notice I had fixed this one. Should work now.

          People

          • Assignee:
            Jesse Kuhnert
            Reporter:
            Raphael Jean
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development