Wicket
  1. Wicket
  2. WICKET-913

PopupCloseLink broken with REDIRECT_TO_RENDER strategy

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.3.0-beta2, 1.3.0-beta3
    • Fix Version/s: 1.3.0-beta4
    • Component/s: wicket
    • Labels:
      None

      Description

      For portlet-support, the only usable render strategy is REDIRECT_TO_RENDER until JSR-286 containers are available.

      So, I'm testing wicket-examples with REDIRECT_TO_RENDER and I noticed the PopupCloseLink in the linkomatic.Popup page example is broken then.
      Note: this also doesn't work in a plain servlet environment either!

      As far as I have been able to discover so far (but I'm not much of an expert in this area), the PopupCloseLink#ClosePopupPage isn't properly recorded in the PageMap when it is set as the responsePage in PopupCloseLink.onClick().

      The request url history from opening the PopupPage to after clicking the close link is as follows:

      • open popup : /wicket-examples/linkomatic/wicket:bookmarkablePage=popuppagemap%3Aorg.apache.wicket.examples.linkomatic.Popup
      • click close: /wicket-examples/linkomatic/wicket:interface=popuppagemap:0:close::ILinkListener::
      • redirect to: /wicket-examples/linkomatic/wicket:interface=popuppagemap:0::::
        ==> Popup is rendered again

      Somehow, removing the current page (Popup) in PopupCloseLink.onClick() from its pagemap doesn't seem to work correctly and/or the #ClosePopupPage isn't properly recorded in the pagemap when it is set as the response page.

      With the default REDIRECT_TO_BUFFER, this problem doesn't surface as the #ClosePopupPage is rendered (to buffer) immediately so it doesn't need to be looked up in the pagemap after the subsequent redirect.

      The weird thing is that if you close the the Popup page directly (not through the "close" link) and thereafter open another instance of the Popup page, everything works fine from then on.
      Maybe this is an indication where the real problem might be, as these subsequent Popup page instances now have a version > 0:

      • click close: /wicket-examples/linkomatic/wicket:interface=popuppagemap:1:close::ILinkListener::
      • redirect to: /wicket-examples/linkomatic/wicket:interface=popuppagemap:0::::
        ==> PopupCloseLink#ClosePopupPage is rendered (which contains <body onLoad="javascript:self.close()">)

      I also checked if this is a new problem since beta2, but it was broken already then too.

        Activity

        Hide
        Matej Knopp added a comment -

        The problem is, that in onClick we do this:

        // Remove the popup's page map from the session
        getPage().getPageMap().remove();

        // Web page closes window using javascript code in PopupCloseLink$1.html
        setResponsePage(new ClosePopupPage());

        But that's wrong for various reasons. ClosePopupPage is stateless page, yet we pass page instance. And also it is created in popup pagemap, which we removed (but re-attached to session again, because of new ClosePopupPage());

        As far as I can tell the fix is to change

        setResponsePage(ClosePopupPage.class);

        Which doesn't require ClosePopupPage instance stored in session (already commited).

        Show
        Matej Knopp added a comment - The problem is, that in onClick we do this: // Remove the popup's page map from the session getPage().getPageMap().remove(); // Web page closes window using javascript code in PopupCloseLink$1.html setResponsePage(new ClosePopupPage()); But that's wrong for various reasons. ClosePopupPage is stateless page, yet we pass page instance. And also it is created in popup pagemap, which we removed (but re-attached to session again, because of new ClosePopupPage()); As far as I can tell the fix is to change setResponsePage(ClosePopupPage.class); Which doesn't require ClosePopupPage instance stored in session (already commited).
        Hide
        Ate Douma added a comment - - edited

        Great, that was quick.
        I've tested it both in normal servlet as well a portlet context and if works of course
        I'll apply this fix to the new 1.3.0-beta3-portlet-support branch too.

        Thanks Matej.

        Show
        Ate Douma added a comment - - edited Great, that was quick. I've tested it both in normal servlet as well a portlet context and if works of course I'll apply this fix to the new 1.3.0-beta3-portlet-support branch too. Thanks Matej.

          People

          • Assignee:
            Matej Knopp
            Reporter:
            Ate Douma
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development