Wicket
  1. Wicket
  2. WICKET-1826

Forms + ModalWindow + AjaxSubmitLink + FormComponent#isInputNullable

    Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.3.3
    • Fix Version/s: None
    • Component/s: wicket, wicket-extensions
    • Labels:
      None

      Description

      Submiting a form which is inside a ModalWindow, wicket javascript sends only the information for the modal window's form, but not for the root form of the page (because ModalWindow hangs its own div at body level).
      On Wicket server side, the form processing is done for the root form, which calls inputChanged for all the components in the page, but the javascript side didn't send the information for them, and then some of them go wrong.
      That happens to FormComponents which have isInputNullable in true.

      More description and proposed solutions in the (to be) attached quickstart project.

      1. modalwindowform.jar
        13 kB
        German Morales
      2. modalwindowform.jar
        13 kB
        German Morales
      3. bug.zip
        26 kB
        Pedro Santos
      4. WICKET-1826.patch
        1 kB
        Pedro Santos

        Activity

        Hide
        Matej Knopp added a comment -

        This shouldn't happen if you have form in your modal window. If that's the case only the form inside modal window should be processed. Also in 1.3 if you have modal window with panel with form, you should always place the modal window inside another form.
        If this problem still persists (in current 1.3 branch) please reopen the issue

        Show
        Matej Knopp added a comment - This shouldn't happen if you have form in your modal window. If that's the case only the form inside modal window should be processed. Also in 1.3 if you have modal window with panel with form, you should always place the modal window inside another form. If this problem still persists (in current 1.3 branch) please reopen the issue
        Hide
        German Morales added a comment -

        >This shouldn't happen if you have form in your modal window.
        I have a form in my modal window, please check the source code.
        Additionally, i'm attaching soon an extended quickstart project that shows how a value entered in the modal window is displayed later in the page (to show that the modal window form is working).

        >If that's the case only the form inside modal window should be processed.
        Well, that's not what seems to be happening, and that's the reason i posted this issue in the first place.

        >Also in 1.3 if you have modal window with panel with form, you should always place the modal window inside another form.
        This phrase is not so clear to me.

        Anyway, i've extended the demo project to include different versions:
        -a modal window inside the page main form
        -another modal window, inside a nested form (inside the page root form)
        -another modal window, inside an additional form (outside the page root form)

        Also, i'm providing 2 checkboxes this time:
        -one inside the page main form
        -an additional inside a nested form (inside the page root form)

        In all cases, after closing any of the modal windows, the checkboxes values are lost.

        Perhaps you can clarify more what's wrong in the demo project?

        Thanks in advance,

        German

        Show
        German Morales added a comment - >This shouldn't happen if you have form in your modal window. I have a form in my modal window, please check the source code. Additionally, i'm attaching soon an extended quickstart project that shows how a value entered in the modal window is displayed later in the page (to show that the modal window form is working). >If that's the case only the form inside modal window should be processed. Well, that's not what seems to be happening, and that's the reason i posted this issue in the first place. >Also in 1.3 if you have modal window with panel with form, you should always place the modal window inside another form. This phrase is not so clear to me. Anyway, i've extended the demo project to include different versions: -a modal window inside the page main form -another modal window, inside a nested form (inside the page root form) -another modal window, inside an additional form (outside the page root form) Also, i'm providing 2 checkboxes this time: -one inside the page main form -an additional inside a nested form (inside the page root form) In all cases, after closing any of the modal windows, the checkboxes values are lost. Perhaps you can clarify more what's wrong in the demo project? Thanks in advance, German
        Hide
        Martin Makundi added a comment -

        Maybe fix it with (I haven't tried the code, though; I would preferably alternatively properly attach the modal window into the effective dom location):

        Form.getRootForm() {
        Form<?> form;

        Form<?> parent = this;
        do

        { form = parent; parent = form.findParent(Form.class); }

        while (parent != null);

        ///////// ModalWindow will have its own root form
        if ((findParent(ModalWindow.class) != null) && (form.findParent(ModalWindow.class) == null))

        { return this; }

        return form;
        }

        Show
        Martin Makundi added a comment - Maybe fix it with (I haven't tried the code, though; I would preferably alternatively properly attach the modal window into the effective dom location): Form.getRootForm() { Form<?> form; Form<?> parent = this; do { form = parent; parent = form.findParent(Form.class); } while (parent != null); ///////// ModalWindow will have its own root form if ((findParent(ModalWindow.class) != null) && (form.findParent(ModalWindow.class) == null)) { return this; } return form; }
        Hide
        Vladimir Kovalyuk added a comment -

        Thanks Martin for the suggestion. The following workaround works:

        Replace your form in ModalWindow with

        public class ModalWindowForm<T> extends Form<T> {
        public ModalWindowForm(String id)

        { super(id); }

        @Override
        public Form<?> getRootForm()

        { Form<?> form = super.getRootForm(); if ((findParent(ModalWindow.class) != null) && (form.findParent(ModalWindow.class) == null)) return this; else return form; }

        }

        What is not working is placing modal window form into another page form, even a root form.

        Show
        Vladimir Kovalyuk added a comment - Thanks Martin for the suggestion. The following workaround works: Replace your form in ModalWindow with public class ModalWindowForm<T> extends Form<T> { public ModalWindowForm(String id) { super(id); } @Override public Form<?> getRootForm() { Form<?> form = super.getRootForm(); if ((findParent(ModalWindow.class) != null) && (form.findParent(ModalWindow.class) == null)) return this; else return form; } } What is not working is placing modal window form into another page form, even a root form.
        Hide
        Matej Knopp added a comment -

        If you have form in modal window make sure that the modal window is placed in another form. Also you need to use latest wicket version for it to work properly.

        Show
        Matej Knopp added a comment - If you have form in modal window make sure that the modal window is placed in another form. Also you need to use latest wicket version for it to work properly.
        Hide
        Vladimir Kovalyuk added a comment -

        The problem is observed with Wicket 1.4-rc2. "make sure that the modal window is placed in another form" - it is true for wicket component tree.
        What particular version do you suggest to check the attachment against?

        Show
        Vladimir Kovalyuk added a comment - The problem is observed with Wicket 1.4-rc2. "make sure that the modal window is placed in another form" - it is true for wicket component tree. What particular version do you suggest to check the attachment against?
        Hide
        Padmaja Kota added a comment -

        I can still the same issue occuring with Wicket 1.3.5 .

        My modal window is like :

        <panel>
        <form>
        remaining form components..
        </form>
        </panel>

        However each time i submit the form the parent form which contains this modal window is also submitted and the inputs on the parent form are set to null causing error in my use case.

        Show
        Padmaja Kota added a comment - I can still the same issue occuring with Wicket 1.3.5 . My modal window is like : <panel> <form> remaining form components.. </form> </panel> However each time i submit the form the parent form which contains this modal window is also submitted and the inputs on the parent form are set to null causing error in my use case.
        Hide
        Peter Ertl added a comment -

        Is this issue existant in 1.4 or 1.5? 1.3 is not maintained anymore.

        Show
        Peter Ertl added a comment - Is this issue existant in 1.4 or 1.5? 1.3 is not maintained anymore.
        Hide
        Pedro Santos added a comment -

        Clément quickstart and patch changing the FormComponent#inputChanged

        About the patch at:
        http://markmail.org/message/5p2cjfxpsxq4sjbi

        Show
        Pedro Santos added a comment - Clément quickstart and patch changing the FormComponent#inputChanged About the patch at: http://markmail.org/message/5p2cjfxpsxq4sjbi
        Hide
        Stijn de Witt added a comment -

        I can confirm this issue is present in Wicket 1.4.21.

        I am a bit surprised by the Minor priority of this issue, as this case doesn't seem as far fetched and when you do get hit by it (as a 3rd party dev using Wicket) it:
        1) Is very hard to figure out what is going wrong.
        2) Can't easily be fixed by yourself because the issue is basically in the Wicket code.

        I intend to try out the patch and report back my findings. With a bit of luck the suggested patch can be easily merged into the next version of Wicket.

        Show
        Stijn de Witt added a comment - I can confirm this issue is present in Wicket 1.4.21. I am a bit surprised by the Minor priority of this issue, as this case doesn't seem as far fetched and when you do get hit by it (as a 3rd party dev using Wicket) it: 1) Is very hard to figure out what is going wrong. 2) Can't easily be fixed by yourself because the issue is basically in the Wicket code. I intend to try out the patch and report back my findings. With a bit of luck the suggested patch can be easily merged into the next version of Wicket.
        Hide
        Stijn de Witt added a comment - - edited

        I could not use the patch, because the code it is altering is in a piece of final Wicket code and I am not allowed to (or willing) to use a custom Wicket version that was built from sources by ourselves.

        So I had a look at the workaround. The instructions for using it are a bit vague... Should we modify the nested form (the one inside the modal window) or the root form (the one on the page itself)? I am not sure. I initially tried with the nested form but the change broke my whole environment. Somewhere it is trying to set the default submit button by calling setDeafultButton, but that method then goed into an infinite loop. It's code calls both isRootForm() and getRootForm(), but in the sugested workaround we override getRootForm in a way that makes it inconsistent with isRootForm. If getRootForm returns this, than isRootForm on this should return true... I altered it so that that is indeed true, and that made it work sorta, but the issue was not fixed by it...

        So I dug deeper and then found a (the) solution.

        First some observations:

        1) Wicket supports nested forms, but HTML does not. To deal with that, Wicket renders divs instead of forms for any nested forms

        2) Wicket's ModalWindow component renders a form tag all of it's own, which looks like this:
        <form style="background-color:transparent;padding:0px;margin:0px;border-width:0px;position:static"><!-- all modal window content is in here, including the nested form DIV --></form>

        3) Wicket's docs state that the modal window should be inside a Form to prevent nested forms from giving issues:
        "If you want to use form in modal window component make sure that you put the modal window itself in another form (nesting forms is legal in Wicket) and that the form on modal window is submitted before the window get closed."
        http://wicket.apache.org/apidocs/1.5/org/apache/wicket/extensions/ajax/markup/html/modal/ModalWindow.html
        If you don't do this the way the docs prescribe, you may get hit by issues such as WICKET-2214

        4) When you do nest the whole modal window inside a Wicket Form, like this:
        ...
        <body>
        ...
        <form wicket:id="modalForm" class="modalForm">
        <div wicket:id="modalWindow">[modalWindow]</div>
        </form>

        <div id="ajax-indicator" class="ajax-indicator">...</div>
        </body>

        You will get HTML that looks like this:

        <form class="modalForm" ... ></form>
        <div class="ajax-indicator">...</div>
        <div class="wicket-modal">
        <form style="background-color:transparent;padding:0px;margin:0px;border-width:0px;position:static">
        <!-- modal window content -->
        <div class="modalwindow" id="form4fa5">
        ...
        <input type="button" onclick="var wcall=wicketSubmitFormById('buttonForm4fa5', ..."/>
        </div>
        </form>
        </div>

        Surprisingly, in the HTML the modal window is not nested inside, but positioned side by side with the modalForm.
        The button's onclick does a wicketSubmitFormById, passing the id of the nested form (which is rendered as a div).
        The implementation of wicketSubmitFormById grabs the div with id "form4fa5" and from there on looks up in the element tree until it finds a form element. It finds the <form style="background-color:transparent;..."> that was rendered by the modal window component. It then grabs all inputs from this form and submits them with an ajax post.

        5) On the server side, the post request is being handled by a Wicket behavior named AjaxFormSubmitBehavior.
        (I am not sure whether this is the only way to do it with ajax, but this is how we are doing it)

        6) That behavior calls onFormSubmitted() on the root form, which calls inputChanged on all it's form components, which try to find their input in the request but fail because only the form generated by the modalwindow component was ever submitted. This is the final cause of this issue.

        Which brings me to the workaround.
        I can't see how the root form can ever get submitted when it is not the same as the nested form, so it seems unreasonable to fire the onFormSubmitted method on it. So I extended AjaxFormSubmitBehavior and made an override for onEvent (which thankfully was not final. Thank you!!) that only calls onFormSubmitted on the nested form. So I just replaced it's first line:

        getForm().getRootForm().onFormSubmitted();

        with:

        getForm().onFormSubmitted();

        This fixes my issue and I don't see any side-effects.

        Hopefully this will help you guys fix form handling for nested forms in dialogs, because it seems it's still broken in Wicket 1.5 as well (don't know about Wicket 6).

        -Stijn

        Also see:

        WICKET-3404: Improve ModalWindow form handling

        Fwd: wicket nested form and modal
        http://apache-wicket.1842946.n4.nabble.com/Fwd-wicket-nested-form-and-modal-td3234777.html

        Show
        Stijn de Witt added a comment - - edited I could not use the patch, because the code it is altering is in a piece of final Wicket code and I am not allowed to (or willing) to use a custom Wicket version that was built from sources by ourselves. So I had a look at the workaround. The instructions for using it are a bit vague... Should we modify the nested form (the one inside the modal window) or the root form (the one on the page itself)? I am not sure. I initially tried with the nested form but the change broke my whole environment. Somewhere it is trying to set the default submit button by calling setDeafultButton, but that method then goed into an infinite loop. It's code calls both isRootForm() and getRootForm(), but in the sugested workaround we override getRootForm in a way that makes it inconsistent with isRootForm. If getRootForm returns this, than isRootForm on this should return true... I altered it so that that is indeed true, and that made it work sorta, but the issue was not fixed by it... So I dug deeper and then found a (the) solution. First some observations: 1) Wicket supports nested forms, but HTML does not. To deal with that, Wicket renders divs instead of forms for any nested forms 2) Wicket's ModalWindow component renders a form tag all of it's own, which looks like this: <form style="background-color:transparent;padding:0px;margin:0px;border-width:0px;position:static"><!-- all modal window content is in here, including the nested form DIV --></form> 3) Wicket's docs state that the modal window should be inside a Form to prevent nested forms from giving issues: "If you want to use form in modal window component make sure that you put the modal window itself in another form (nesting forms is legal in Wicket) and that the form on modal window is submitted before the window get closed." http://wicket.apache.org/apidocs/1.5/org/apache/wicket/extensions/ajax/markup/html/modal/ModalWindow.html If you don't do this the way the docs prescribe, you may get hit by issues such as WICKET-2214 4) When you do nest the whole modal window inside a Wicket Form, like this: ... <body> ... <form wicket:id="modalForm" class="modalForm"> <div wicket:id="modalWindow"> [modalWindow] </div> </form> <div id="ajax-indicator" class="ajax-indicator">...</div> </body> You will get HTML that looks like this: <form class="modalForm" ... ></form> <div class="ajax-indicator">...</div> <div class="wicket-modal"> <form style="background-color:transparent;padding:0px;margin:0px;border-width:0px;position:static"> <!-- modal window content --> <div class="modalwindow" id="form4fa5"> ... <input type="button" onclick="var wcall=wicketSubmitFormById('buttonForm4fa5', ..."/> </div> </form> </div> Surprisingly, in the HTML the modal window is not nested inside, but positioned side by side with the modalForm. The button's onclick does a wicketSubmitFormById, passing the id of the nested form (which is rendered as a div). The implementation of wicketSubmitFormById grabs the div with id "form4fa5" and from there on looks up in the element tree until it finds a form element. It finds the <form style="background-color:transparent;..."> that was rendered by the modal window component. It then grabs all inputs from this form and submits them with an ajax post. 5) On the server side, the post request is being handled by a Wicket behavior named AjaxFormSubmitBehavior. (I am not sure whether this is the only way to do it with ajax, but this is how we are doing it) 6) That behavior calls onFormSubmitted() on the root form, which calls inputChanged on all it's form components, which try to find their input in the request but fail because only the form generated by the modalwindow component was ever submitted. This is the final cause of this issue. Which brings me to the workaround. I can't see how the root form can ever get submitted when it is not the same as the nested form, so it seems unreasonable to fire the onFormSubmitted method on it. So I extended AjaxFormSubmitBehavior and made an override for onEvent (which thankfully was not final. Thank you!!) that only calls onFormSubmitted on the nested form. So I just replaced it's first line: getForm().getRootForm().onFormSubmitted(); with: getForm().onFormSubmitted(); This fixes my issue and I don't see any side-effects. Hopefully this will help you guys fix form handling for nested forms in dialogs, because it seems it's still broken in Wicket 1.5 as well (don't know about Wicket 6). -Stijn Also see: WICKET-3404 : Improve ModalWindow form handling Fwd: wicket nested form and modal http://apache-wicket.1842946.n4.nabble.com/Fwd-wicket-nested-form-and-modal-td3234777.html
        Hide
        Stijn de Witt added a comment -

        Can we upgrade the priority of this issue? It breaks core functionality. Major seems a lot more suitable for it.

        Show
        Stijn de Witt added a comment - Can we upgrade the priority of this issue? It breaks core functionality. Major seems a lot more suitable for it.
        Hide
        German Morales added a comment - - edited

        Hi Stijn,

        it's nice to see someone finally interested in fixing this bug.

        I hope you didn't lose much time with it, but what you propose...
        >replaced it's first line:
        >getForm().getRootForm().onFormSubmitted();
        >with:
        >getForm().onFormSubmitted();
        is exactly what i've already proposed back on Sep/08 (see the attached modalwindowform.jar, MyAjaxSubmitLink.java, line 41.
        >protected void onEvent(AjaxRequestTarget cTarget) {
        > getForm().onFormSubmitted(); // HERE IS THE CHANGE: before it was this way... getForm().getRootForm().onFormSubmitted();
        and after 4 years it's apparently still not accepted.

        i hope we have more luck this time

        Best regards,
        German

        Show
        German Morales added a comment - - edited Hi Stijn, it's nice to see someone finally interested in fixing this bug. I hope you didn't lose much time with it, but what you propose... >replaced it's first line: >getForm().getRootForm().onFormSubmitted(); >with: >getForm().onFormSubmitted(); is exactly what i've already proposed back on Sep/08 (see the attached modalwindowform.jar, MyAjaxSubmitLink.java, line 41. >protected void onEvent(AjaxRequestTarget cTarget) { > getForm().onFormSubmitted(); // HERE IS THE CHANGE: before it was this way... getForm().getRootForm().onFormSubmitted(); and after 4 years it's apparently still not accepted. i hope we have more luck this time Best regards, German
        Hide
        Stijn de Witt added a comment -

        Hi German. Oh wow I missed that somehow!
        But at least it is reassuring that we both came to the same conclusion. It reinforces my idea that it is a valid solution.

        I don't think the Wicket developers use nested forms much. That is too bad but alas, that is life.
        Of course it's not very encouraging if your patch never makes it. It will make you think twice before submitting one in the future...
        Let's hope this gets picked up by someone this time.

        Show
        Stijn de Witt added a comment - Hi German. Oh wow I missed that somehow! But at least it is reassuring that we both came to the same conclusion. It reinforces my idea that it is a valid solution. I don't think the Wicket developers use nested forms much. That is too bad but alas, that is life. Of course it's not very encouraging if your patch never makes it. It will make you think twice before submitting one in the future... Let's hope this gets picked up by someone this time.
        Hide
        Martin Grigorov added a comment -

        Hi,

        The problem definitely wont be fixed in 1.4.x branch. 1.4.x is frozen and only security fixes go in.
        Please update the quickstart and the patch to Wicket 6.3.0 and we will consider it.
        Thank you!

        Show
        Martin Grigorov added a comment - Hi, The problem definitely wont be fixed in 1.4.x branch. 1.4.x is frozen and only security fixes go in. Please update the quickstart and the patch to Wicket 6.3.0 and we will consider it. Thank you!
        Hide
        Vladimir Kovalyuk added a comment -

        >> I don't think the Wicket developers use nested forms much. That is too bad but alas, that is life.

        Common! It is a great capability making Wicket best in breed

        Show
        Vladimir Kovalyuk added a comment - >> I don't think the Wicket developers use nested forms much. That is too bad but alas, that is life. Common! It is a great capability making Wicket best in breed

          People

          • Assignee:
            Unassigned
            Reporter:
            German Morales
          • Votes:
            7 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:

              Development