Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draggable/Droppable: submits Ajax requests via the wrong form #3265

Closed
stiemannkj1 opened this issue Jan 31, 2018 · 13 comments
Closed

Draggable/Droppable: submits Ajax requests via the wrong form #3265

stiemannkj1 opened this issue Jan 31, 2018 · 13 comments
Labels
enhancement Additional functionality to current component 🐞 defect Bug...Something isn't working
Milestone

Comments

@stiemannkj1
Copy link
Contributor

1) Environment

  • PrimeFaces versions: 6.1, 6.2.RC1, master
  • Does it work on the newest released PrimeFaces version? The bug exists in the latest version.
  • Does it work on the newest sources in GitHub? The bug exists in master.
  • Application server + version: Any
  • Affected browsers: Firefox, Chrome

2) Expected behavior

Draggable/Droppable components submit Ajax requests via the h:form component that they are contained in.

3) Actual behavior

Draggable/Droppable components submit Ajax requests via the first h:form component on the page.

Since Draggable/Droppable never actually renders an HTML element with its clientId, the following code in core.ajax.js never finds the source element:

var $source = $(PrimeFaces.escapeClientId(sourceId));

Since the element is not found, the parent form cannot be found in the next line:

form = $source.closest('form');

So the code grabs the first form on the page as a fallback:

if (form.length === 0) {
    form = $('form').eq(0);
}

This causes serious problems in Liferay where this code may end up attempting to submit an Ajax request via a completely different portlet.

4) Workaround and Potential Solution(s)

There is a simple workaround that works in Liferay: specify the id of the form you are using for the Ajax request:

<h:form binding="#{form}">
    <!-- Your code here.... -->
    <p:droppable for="selectedCars" tolerance="touch" activeStyleClass="ui-state-highlight"
        datasource="availableCars">
        <p:ajax listener="#{dndCarsView.onCarDrop}" update="dropArea availableCars" form="#{form.clientId}" />
    </p:droppable>
</h:form>

One potential solution would be to render an empty span with the correct id for each p:droppable, but there may be other components that suffer from this problem and they would need to follow this solution too. However, this would add needless markup to the page.

Alternatively, it might be possible to take the invalid id and extract a valid parent id from it (inside core.ajax.js):

// Use UINamingContainer.getSeparatorChar() on the server to get the actual separator char.
var separatorChar = ":";
var invalidId = "formId:validParentId:invalidId";
invalidId.substring(0, invalidId.lastIndexOf(separatorChar));
// Repeat until a valid form is found or the base id has been searched and is considered invalid.

Perhaps the best solution would be so simply encode the form id for every Ajax behavior. That would fix this issue for all components that might be affected, however it does seem like a big fix for this problem.

5) Steps to reproduce

  1. Add the following XHTML and Bean.java to a webapp and navigate to the XHTML page.
  2. Open the browser console.
  3. Drag the "1" row from the first dataTable into the first drop area.
  4. "Submitted via form1" will appear in the browser console.
  5. Drag the "a" row from the second dataTable into the second drop area.
  6. "Submitted via form1" will appear in the browser console.

6) Sample XHTML

<?xml version="1.0" encoding="UTF-8"?>
<f:view xmlns="http://www.w3.org/1999/xhtml" 
	xmlns:f="http://xmlns.jcp.org/jsf/core" xmlns:h="http://xmlns.jcp.org/jsf/html" 
	xmlns:p="http://primefaces.org/ui">
	<h:head />
	<h:body styleClass="jsf-applicant-portlet ltr">
		<h:form id="form1" style="border: black; border-style: solid;">

			<h1>form1</h1>
			<p:dataTable id="draggableList" var="draggableString" value="#{bean.draggableList1}">
				<p:column style="width:20px">
					<h:outputText id="dragIcon" styleClass="ui-icon ui-icon-arrow-4" />
					<p:draggable for="dragIcon" revert="true" helper="clone"/>
				</p:column>

				<p:column headerText="draggableString">
					<h:outputText value="#{draggableString}" />
				</p:column>
			</p:dataTable>

			<p:fieldset id="dropArea" legend="Dropped Strings" style="margin-top:20px">
				<p:outputPanel id="dropPanel">
					<h:outputText value="!!!Drop here!!!" rendered="#{empty bean.droppedList1}"
						style="font-size:24px;" />
					<p:dataTable id="droppableList" var="droppedString" value="#{bean.droppedList1}"
						rendered="#{not empty bean.droppedList1}">
						<p:column headerText="droppedString">
							<h:outputText value="#{droppedString}" />
						</p:column>
					</p:dataTable>
				</p:outputPanel>
			</p:fieldset>

			<p:droppable for="dropArea" tolerance="touch" activeStyleClass="ui-state-highlight"
				datasource="draggableList">
				<p:ajax listener="#{bean.onDrop1}" update="dropPanel draggableList" onstart="logFormId(arguments[0]);" />
			</p:droppable>

		</h:form>
		<h:form id="form2" style="border: black; border-style: solid;">

			<h1>form2</h1>
			<p:dataTable id="draggableList" var="draggableString" value="#{bean.draggableList2}">
				<p:column style="width:20px">
					<h:outputText id="dragIcon" styleClass="ui-icon ui-icon-arrow-4" />
					<p:draggable for="dragIcon" revert="true" helper="clone"/>
				</p:column>

				<p:column headerText="draggableString">
					<h:outputText value="#{draggableString}" />
				</p:column>
			</p:dataTable>

			<p:fieldset id="dropArea" legend="Dropped Strings" style="margin-top:20px">
				<p:outputPanel id="dropPanel">
					<h:outputText value="!!!Drop here!!!" rendered="#{empty bean.droppedList2}"
						style="font-size:24px;" />
					<p:dataTable id="droppableList" var="droppedString" value="#{bean.droppedList2}"
						rendered="#{not empty bean.droppedList2}">
						<p:column headerText="droppedString">
							<h:outputText value="#{droppedString}" />
						</p:column>
					</p:dataTable>
				</p:outputPanel>
			</p:fieldset>

			<p:droppable for="dropArea" tolerance="touch" activeStyleClass="ui-state-highlight"
				datasource="draggableList">
				<p:ajax listener="#{bean.onDrop2}" update="dropPanel draggableList" onstart="logFormId(arguments[0]);" />
			</p:droppable>

		</h:form>
		<h:outputScript>
			function logFormId(data) {
				var $source = $(PrimeFaces.escapeClientId(data.source));
				//look for a parent of source
				var form = $source.closest('form');

				//source has no parent form so use first form in document
				if (form.length === 0) {
					form = $('form').eq(0);
				}

				console.log('Submitted with ' + form[0].id);
			}
		</h:outputScript>
	</h:body>
</f:view>

7) Sample bean

@ManagedBean
@ViewScoped
public class Bean {

	private List<String> draggableList1 = new ArrayList<String>(Arrays.asList("1", "2", "3"));
	private List<String> droppedList1 = new ArrayList<String>();
	private List<String> draggableList2 = new ArrayList<String>(Arrays.asList("a", "b", "c"));
	private List<String> droppedList2 = new ArrayList<String>();

	public List<String> getDraggableList1() {
		return draggableList1;
	}

	public void setDraggableList1(List<String> draggableList1) {
		this.draggableList1 = draggableList1;
	}

	public List<String> getDroppedList1() {
		return droppedList1;
	}

	public void setDroppedList1(List<String> droppedList1) {
		this.droppedList1 = droppedList1;
	}

	public List<String> getDraggableList2() {
		return draggableList2;
	}

	public void setDraggableList2(List<String> draggableList2) {
		this.draggableList2 = draggableList2;
	}

	public List<String> getDroppedList2() {
		return droppedList2;
	}

	public void setDroppedList2(List<String> droppedList2) {
		this.droppedList2 = droppedList2;
	}

    public void onDrop1(DragDropEvent ddEvent) {

        String string = ((String) ddEvent.getData());
        droppedList1.add(string);
        draggableList1.remove(string);
    }

    public void onDrop2(DragDropEvent ddEvent) {

        String string = ((String) ddEvent.getData());
        droppedList2.add(string);
        draggableList2.remove(string);
    }
}
@tandraschko tandraschko changed the title PrimeFaces Draggable/Droppable submits Ajax requests via the wrong form Draggable/Droppable: submits Ajax requests via the wrong form Feb 1, 2018
@tandraschko tandraschko added 🐞 defect Bug...Something isn't working enhancement Additional functionality to current component labels Feb 1, 2018
@stiemannkj1
Copy link
Contributor Author

@melloware
Copy link
Member

@stiemannkj1 Excellent debugging.

@tandraschko
Copy link
Member

@stiemannkj1 I think that we should filter out forms without viewstate in our lookup. Should be fine for JSF and Liferay, right?

@stiemannkj1
Copy link
Contributor Author

@tandraschko, that still won't work because there could be two JSF portlets on the same page.

@tandraschko
Copy link
Member

Yep, i see.
Lets talk about it again after we released 6.2.

@tandraschko tandraschko added this to the 6.3 milestone Feb 1, 2018
@stiemannkj1
Copy link
Contributor Author

@tandraschko sounds good, as I said in the issue: this problem could affect other components as well (if they don't render any markup). But the workaround is very nice and simple and this problem doesn't seem to affect any extremely popular components, so I think the fix can be safely delayed.

@tandraschko
Copy link
Member

We could also render the script tag with the clientId instead of clientId + "_s"
Same like for RemoteCommand

@stiemannkj1
Copy link
Contributor Author

@tandraschko, that seems like a good solution. Can you check for other components that might suffer from this problem (meaning that they don't render any markup with their default client id) and apply the same solution for them?

tandraschko added a commit that referenced this issue Jul 23, 2018
@tandraschko
Copy link
Member

@stiemannkj1 Could you give it a try?

@tandraschko
Copy link
Member

Fuck, that doesn't work with MOVE_SCRIPTS_TO_BOTTOM
we have to find another solution

tandraschko added a commit that referenced this issue Jul 23, 2018
@tandraschko
Copy link
Member

We render a dummy markup now.

@stiemannkj1
Copy link
Contributor Author

@tandraschko, I'll check it tomorrow. Sorry, I've been busy with other tasks.

@stiemannkj1
Copy link
Contributor Author

stiemannkj1 commented Aug 10, 2018

@tandraschko, thanks for fixing this! It works correctly in Liferay now. Did you happen to check for other components (that render no markup) that might suffer from this problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additional functionality to current component 🐞 defect Bug...Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants