Nested forms check fails with <p:dialog appendTo="@(body)"> #349

Closed
julesy89 opened this Issue Jan 27, 2017 · 10 comments

Projects

None yet

2 participants

@julesy89

I have a problem regarding the #181 nested form check.
Using the primefaces component <p:dialog> with the appendTo="@(body)" attribute adds the dialog via JS to the body element.
However, the nested form check fails, because - I assume - it is done before the JS function finished.
Is there any workaround for that issue instead of setting PROJECT_STAGE to Production? When exactly is validateComponentTreeStructure called?

Example:

<h:form id="form">
    <p:commandButton id="btn-show" type="button" onclick="PF('dlg').show();" title="show"/>
    <p:dialog id="dlg" widgetVar="dlg" dynamic="true" appendTo="@(body)">
        <h:form id="dlgForm">
            <p:commandButton id="btn-close" type="button" onclick="PF('dlg').hide();" title="close"/>
        </h:form>
    </p:dialog>
</h:form>

Error in Development:
java.lang.IllegalStateException: Nested form with ID 'form:dlgForm' encountered inside parent form with ID 'form'. This is illegal in HTML.

Rendered as:

<form id="form" name="form" method="post" enctype="application/x-www-form-urlencoded">
    <input type="hidden" name="form" value="form">
    <button id="form:btn-show" name="form:btn-show" class="" onclick="PF('dlg').show();" title="show"    type="button" role="button" aria-disabled="false"><span class="ui-button-text ui-c">show</span>    </button><input type="hidden" name="javax.faces.ViewState" id="j_id1:javax.faces.ViewState:0"   value="-2231695417190970878:-5176726235829428485" autocomplete="off">
</form>

<div id="form:dlg" class="ui-dialog ui-widget ui-widget-content ui-corner-all ui-shadow ui-hidden-container ui-draggable ui-resizable" style="width: auto; height: auto; left: 741.5px; top: 441px; z-index: 1011; display: block;" role="dialog" aria-labelledby="form:dlg_title" aria-hidden="false" aria-live="polite"><div class="ui-dialog-titlebar ui-widget-header ui-helper-clearfix ui-corner-top ui-draggable-handle"><span id="form:dlg_title" class="ui-dialog-title"></span><a href="#" class="ui-dialog-titlebar-icon ui-dialog-titlebar-close ui-corner-all" aria-label="Close" role="button"><span class="ui-icon ui-icon-closethick"></span></a></div><div class="ui-dialog-content ui-widget-content" style="height: auto;">
    <form id="form:dlgForm" name="form:dlgForm" method="post" enctype="application/x-www-form-urlencoded">
    <input type="hidden" name="form:dlgForm" value="form:dlgForm">
    <button id="form:dlgForm:btn-close" name="form:dlgForm:btn-close" class="" onclick="PF('dlg').hide();" title="close" type="button" role="button" aria-disabled="false"><span class="ui-button-text ui-c">close</span></button>
    <input type="hidden" name="javax.faces.ViewState" value="-2231695417190970878:-5176726235829428485" autocomplete="off"></form></div><div class="ui-resizable-handle ui-resizable-n" style="z-index: 90;"></div><div class="ui-resizable-handle ui-resizable-s" style="z-index: 90;"></div><div class="ui-resizable-handle ui-resizable-e" style="z-index: 90;"></div><div class="ui-resizable-handle ui-resizable-w" style="z-index: 90;"></div><div class="ui-resizable-handle ui-resizable-ne" style="z-index: 90;"></div><div class="ui-resizable-handle ui-resizable-nw" style="z-index: 90;"></div><div class="ui-resizable-handle ui-resizable-se ui-icon ui-icon-gripsmall-diagonal-se" style="z-index: 90;"></div><div class="ui-resizable-handle ui-resizable-sw" style="z-index: 90;"></div>
</div>

Note: Without the p:dialog attribute dynamic="true", the inner form is rendered as:

<div class="ui-dialog-content ui-widget-content" style="height: auto;">
<input type="hidden" name="form:dlgForm" value="form:dlgForm">
<button id="form:dlgForm:btn-close" name="form:dlgForm:btn-close" class="" onclick="PF('dlg').hide();" title="close" type="button" role="button" aria-disabled="false"><span class="ui-button-text ui-c">close</span></button><input type="hidden" name="javax.faces.ViewState" id="j_id1:javax.faces.ViewState:0" value="-4969891793333898230:-9196376097730976112" autocomplete="off">
</div>

The form component is not present explicitly. The nested form check still fails.

Very best and thanks in advance,
Julian

@BalusC
Member
BalusC commented Jan 31, 2017

Is there any technical reason why you don't just move out the nested form?

@julesy89
julesy89 commented Jan 31, 2017 edited

Not directly technical. But our application is component based and some components do have dialogs inside.

For instance, we have different picker components to open a dialog in order to pick one out of multiple values.

picker_example

By clicking at the zoom button a dialog shows a list of charges. The whole component can be parameterized to filter values beforehand. A listener is used to get the current selected item.

In general, we include multiple components:

<h:form>
    <my:component1></my:component1>
    <my:component2></my:component2>
    <my:component3></my:component3>
    <p:commandButton value="button" type="button"></p:commandButton>
</h:form>

my:componentX defined as:

<p:outputLabel value="X" />
<p:autoComplete></p:autoComplete>
<p:commandButton id="browse" oncomplete="dlg.show();" ></p:commandButton>
<p:commandButton id="clear"></p:commandButton>
<p:dialog id="dlg" appendTo="@(body)">
    <h:form>
    ....
    </h:form>
</p:dialog>

However, we are using this kind of components throughout our whole application. We didn't see any problem so far, since all p:dialogs are appended directly to the body.
If there is any other simply workaround or design pattern to apply, please let us know.

@BalusC BalusC closed this in 0215e13 Jan 31, 2017
@BalusC BalusC added a commit that referenced this issue Jan 31, 2017
@BalusC BalusC #349: utilize existing utility 8665d2a
@BalusC
Member
BalusC commented Jan 31, 2017

Reasonable. I added a bypass on the check when the nested form is in turn nested in a component (subclass) of p:dialog. The fix is available in today's latest 2.6-SNAPSHOT. Please let me know whether that works for you.

@julesy89

Thank you for your fast answer and your fix. It works perfectly!

Nevertheless, I checked meanwhile all our forms and dialogs manually and found one case where we did not follow the nested form constraint and used two nested forms in ONE dialog.
The nested form check does not complain about nested forms since they are wrapped by a p:dialog component.

<p:dialog>
<h:form id="outer"><h:form id="inner"></h:form></h:form>
</p:dialog>

It might make sense to switch back to your first commit and fix it with:

public static boolean isNestedInPrimeFacesDialogWithNoFormInBetween(UIComponent component) {
        if (PRIMEFACES_DIALOG_CLASS != null) {
            for (UIComponent parent = component.getParent(); parent != null; parent = parent.getParent()) {
                if (UIForm.class.isInstance(parent)) {
                    return false;
                } else if (PRIMEFACES_DIALOG_CLASS.isAssignableFrom(parent.getClass())) {
                    return true;
                }
            }
        }
        return false;
}

or include another form in between check.

But that is only an additional hint. Again thank you for seeing the need to fix it!

@BalusC
Member
BalusC commented Jan 31, 2017

Right, I fixed it.

@julesy89
julesy89 commented Feb 1, 2017 edited

Sorry to say: The latest fix does not work for us. Sometimes a component uses another component.

The second fix forbids:

<p:dialog id="outer-dlg">
            <h:form id="outer">
                <p:dialog id="inner-dlg">
                    <h:form id="inner"></h:form>
                </p:dialog>
            </h:form>
</p:dialog>

java.lang.IllegalStateException: Nested form with ID 'outer:inner' encountered inside parent form with ID 'outer'. This is illegal in HTML.

which is indeed - considering dialogs are appended to body - correct.

nestedParent := outer
form := inner

evaluates to

(nestedParent != null && 
     (!Hacks.isNestedInPrimeFacesDialog(form) || Hacks.isNestedInPrimeFacesDialog(nestedParent))) 

(true && (false || true) => true =>  throw new IllegalStateException
			
@BalusC
Member
BalusC commented Feb 1, 2017

Well, those are awkward constructs.

@julesy89
julesy89 commented Feb 1, 2017 edited

You are right, it seems to be awkward at first glance. But for us it is nothing else than using nested components which do have dialogs and forms.

We would also be satisfied with having the option to turn off the nested form check completely.

@BalusC
Member
BalusC commented Feb 1, 2017

Let me know if current 2.6-SNAPSHOT does it for you.

@julesy89
julesy89 commented Feb 2, 2017

Yes, it does it! All my testcases are working or showing the intended error message.

It took me some minutes to comprehend Hacks.isNestedInPrimeFacesDialog(form, nestedParent), but it is quite elegant ;)

Thanks for your efforts and time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment