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

Picklist: filter is causing script error and browser crash for more than 4000 items #4320

Closed
Amit8608525622 opened this Issue Dec 10, 2018 · 18 comments

Comments

Projects
None yet
5 participants
@Amit8608525622
Copy link

Amit8608525622 commented Dec 10, 2018

IMPORTANT :

Picklist filter is not working properly.It is taking too much time to filter the data.We have tried to change the browser setting to resolve this issue like setting the value of dom.max_chrome_script_run_time;0 and
dom.max_script_run_time;0, but it didn't help.It just avoided the script error message but browser was still getting crashed and it is taking 2 min 47 seconds to filter data from 4000 items..
It is an very urgent issue for me. Could anyone please look into it and provide me a solution or workaround for it ASAP.

1) Environment

  • PrimeFaces version:6.2.9./6.2.12.(In 6.1 it works properly)
  • Does it work on the newest released PrimeFaces version?No Version?6.2.12
  • Does it work on the newest sources in GitHub?No (Build by source -> https://github.com/primefaces/primefaces/wiki/Building-From-Source)
  • Application server + version: Tomcat 7.0
  • Affected browsers: All

2) Expected behavior: After typing in picklist filter, it should filter the value from picklist.It should not take much time to filter and browser should not crash.

...

3) Actual behavior: While filtering the picklist, it is causing script error(see attachment). If i click on continue , it is causing browser crash(Mozila Firefox 52.0).

4) Steps to reproduce :

1- Add more than 4000 items in the picklist (primeface version 6.2.9 or 6.2.12).
2- Add showSourceFilter="true" showTargetFilter="true" filterMatchMode="contains".
3- Try to filter the picklist items using filter text area.
4 - It will take approx 3 minutes to filter the picklist or script error(see attachment) or Browser crash.

..

5) Sample XHTML--

<p:pickList id="pickList"  value="#{userActivation.users}" var="users" showCheckbox="true" effect="none" itemLabel="#{users}"  itemValue="#{users}" showSourceFilter="true" showTargetFilter="true" filterMatchMode="contains" filterEvent="enter">
		<p:ajax event="transfer" onstart="PF('bui').show()" oncomplete="PF('bui').hide()"> </p:ajax>
			
								
			<f:facet name="sourceCaption">#{msg['msg.inactiveUser']}</f:facet>
        	<f:facet name="targetCaption">#{msg['facet.ActiveUsers']}</f:facet>
        	
        	<p:column style="width:10%">
								<h:outputLabel id="img_#{users}" styleClass="ui-user"/>
			</p:column>
			<p:column style="width:90%">
							<h:outputLabel id="txt_#{users}" value="#{users}"/>
			</p:column>
        </p:pickList>

..

6) Sample bean--

@ManagedBean
@ViewScoped
public class UserActivation {

	/**
	 * Logger Object to print log
	 */
	private static Logger logger = Logger
			.getLogger("");

	private DualListModel<String> users = new DualListModel<String>(
			Collections.<String> emptyList(), Collections.<String> emptyList());
	private List<String> usersSource;
	private List<String> usersTarget;
	
	@PostConstruct
	public void init() {
		final String lBuf = " ";
		logger.info(lBuf + "");
		try {
			IDfSession session = SessionManager.instance()
					.getWEBSession();

			
			String superUserID = DAOFactory
					.getContextConfig()
					.getString(Constants.DOCBASE_SUPERUSER_NAME).trim();
			
			AdminAPI adminApi = new AdminAPI();
			
			String superUserName= adminApi.getFullNameByUserID(superUserID, session);
			usersSource = adminApi.getUsersAllByGroupName(
					AdminConstants.grp_global_none, session);
			usersTarget = adminApi.getUsersAllByGroupName(
					AdminConstants.grp_global_users, session);
			if(usersTarget.contains(session.getLoginUserName())){
				usersTarget.remove(session.getLoginUserName());
				usersTarget.remove(superUserName);
			}
			if (usersSource == null) {
				usersSource = new ArrayList<String>(
						Collections.<String> emptyList());
			}
			if (usersTarget == null) {
				usersTarget = new ArrayList<String>(
						Collections.<String> emptyList());
			}
			users = new DualListModel<String>(usersSource, usersTarget);
		} catch (AdminException exp) {
			logger.error(lBuf + exp);
		} catch (Exception exp) {
			logger.error(lBuf + exp);
		} finally {
			logger.info(lBuf + "");
		}
	}
}	

Please let me know if you need any further information.

@Amit8608525622

This comment has been minimized.

Copy link

Amit8608525622 commented Dec 10, 2018

Please try on browser Mozilla Firefox 52.9.0 (32-bit) and IE as well.

@tandraschko tandraschko changed the title Picklist : picklist filter is causing script error and browser crash(Mozila Firefox 52.0) for more than 4000 items added in picklist. Picklist: filter is causing script error and browser crash for more than 4000 items Dec 10, 2018

@tandraschko

This comment has been minimized.

Copy link
Member

tandraschko commented Dec 10, 2018

i labbeled it as enhancement as picklist was never designed for such a big lists.
Feel free to analyze the performance and provide a PR.

@Rapster

This comment has been minimized.

Copy link
Member

Rapster commented Dec 10, 2018

Since it has been already loaded I don't see why it would take so much time (e.g 2 min 47 seconds) to filter just 4000 items.... Please provide a sample as the issue template stated, this is the only way to analyse your problem.

@Rapster

This comment has been minimized.

Copy link
Member

Rapster commented Dec 10, 2018

FYI, in the future please make an effort about the format/indentation of your posted code 😉

@melloware

This comment has been minimized.

Copy link
Contributor

melloware commented Dec 10, 2018

I know we fixed this Picklist Performance Issue in 6.2.7 which turned out to be a JQuery 3 related issue. #3675 I wonder if the filter is a similar issue.

@Rapster

This comment has been minimized.

Copy link
Member

Rapster commented Dec 10, 2018

Thanks Emil, I tried to find that issue as I remembered we faced a similar issue.

@melloware

This comment has been minimized.

Copy link
Contributor

melloware commented Dec 10, 2018

Attached is a demo showing the issue with 4000 items.
pf-4320.zip

@Amit8608525622

This comment has been minimized.

Copy link

Amit8608525622 commented Dec 10, 2018

Below is the sample code to reporduce this issue-

Bean -

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import javax.annotation.PostConstruct;
import javax.faces.bean.ManagedBean;
import javax.faces.bean.ViewScoped;

import org.primefaces.model.DualListModel;

@ManagedBean
@ViewScoped
public class SampleUserActivation {

private DualListModel<String> users = new DualListModel<String>(
		Collections.<String> emptyList(), Collections.<String> emptyList());
private List<String> usersSource = new ArrayList<>();;
private List<String> usersTarget = new ArrayList<>();

public List<String> getUsersTarget() {
	return usersTarget;
}

public void setUsersTarget(List<String> usersTarget) {
	this.usersTarget = usersTarget;
}

public DualListModel<String> getUsers() {
	return users;
}

public void setUsers(DualListModel<String> users) {
	this.users = users;
}

@PostConstruct
public void init() {
	try {
		for (int i = 0; i < 4000; i++) {
			usersSource.add("UserSource " + i);
			usersTarget.add("UserTarget " + i);
		}

		users = new DualListModel<String>(usersSource, usersTarget);
	} catch (Exception exp) {
	} finally {
		System.out.println("Method Exits");
	}
}

}

XHTML code--

<h:head>
</h:head>
<h:body id="sampleUserActivation">
<p:pickList id="pickList" value="#{sampleUserActivation.users}"
var="users" showCheckbox="true" effect="none" itemLabel="#{users}"
itemValue="#{users}" showSourceFilter="true" showTargetFilter="true"
filterMatchMode="contains" filterEvent="enter">
<p:ajax event="transfer" onstart="PF('bui').show()"
oncomplete="PF('bui').hide()">
</p:ajax>

	<f:facet name="sourceCaption">#{msg['msg.inactiveUser']}</f:facet>
	<f:facet name="targetCaption">#{msg['facet.ActiveUsers']}</f:facet>

	<p:column style="width:10%">
		<h:outputLabel id="img_#{users}" styleClass="ui-user" />
	</p:column>
	<p:column style="width:90%">
		<h:outputLabel id="txt_#{users}" value="#{users}" />
	</p:column>
</p:pickList>

</h:body>

Can we have a workaround for this issue as it is needed to work for my project. I will be very grateful to you.

melloware added a commit to melloware/primefaces that referenced this issue Dec 10, 2018

Fix primefaces#4320: Picklist performance improvement. Remove animati…
…on effect and move UI update outside of for loop.
@melloware

This comment has been minimized.

Copy link
Contributor

melloware commented Dec 10, 2018

PR Submitted: #4321

The biggest issue was the work to update the UI was being done inside the for loop as well as trying to animate it. My fix has the filter taking about 1 second for 4000 items.

@Amit8608525622

This comment has been minimized.

Copy link

Amit8608525622 commented Dec 11, 2018

Can you please let me know when and in which version this fix is going to release and available for us?

@tandraschko

This comment has been minimized.

Copy link
Member

tandraschko commented Dec 11, 2018

just follow the ticket here.... ;)

@Amit8608525622

This comment has been minimized.

Copy link

Amit8608525622 commented Dec 11, 2018

Can I have the compiled jar file for this fix. I really need it for my project.

@Rapster

This comment has been minimized.

Copy link
Member

Rapster commented Dec 11, 2018

You'll have to compile by yourself. See the wiki

@tandraschko

This comment has been minimized.

Copy link
Member

tandraschko commented Dec 11, 2018

we didn't fix anything currently.

@Amit8608525622

This comment has been minimized.

Copy link

Amit8608525622 commented Dec 11, 2018

The fix(melloware@e09d8ca) provided by the @melloware is solving the issue? I am not able to test it as there are lot of dependencies needed to compile the source code and it needs license to get those dependencies.

@tandraschko

This comment has been minimized.

Copy link
Member

tandraschko commented Dec 11, 2018

probably but we didn't apply it yet.
Please learn about how such a open source project works. If we didn't commit anything / the PR isn't closed, it's not yet fixed.
Even if we fix it, it's just commited here in the community version, which will become 6.3.
6.2.x (PrimeFaces ELITE) is not open source and managed by PrimeTek only.

@melloware

This comment has been minimized.

Copy link
Contributor

melloware commented Dec 11, 2018

@Amit8608525622 If you need this fix right now simply add this to your JS file that gets included AFTER PrimeFaces. This will override the filter function in the widget with my fix. This will at least get you going right now.

PrimeFaces.widget.PickLis.prototype.filter = function(value, list) {
        var filterValue = $.trim(value).toLowerCase(),
        items = list.children('li.ui-picklist-item'),
        animated = this.isAnimated(),
        $this = this;

        if(filterValue === '') {
            items.filter(':hidden').show();
        }
        else {
            for(var i = 0; i < items.length; i++) {
                var item = items.eq(i),
                itemLabel = item.attr('data-item-label'),
                matches = this.filterMatcher(itemLabel, filterValue);

                if(matches) {
                    item.show();
                }
                else {
                    item.hide();
                }
            }
            
            this.updateListRole();
        }
    }

This is the power of PrimeFaces and JS. You can override almost anything.

@mertsincan mertsincan added the 6.2.14 label Dec 13, 2018

@mertsincan mertsincan self-assigned this Dec 13, 2018

@mertsincan mertsincan added this to the 6.3 milestone Dec 13, 2018

@mertsincan

This comment has been minimized.

Copy link
Member

mertsincan commented Dec 13, 2018

Fixed for 6.2.14 and 6.3. For now, you can try the following js code;

<script type="text/javascript">
//<![CDATA[
 /**
 * PrimeFaces PickList Widget
 */
PrimeFaces.widget.PickList.prototype.filter = function(value, list) {
    var filterValue = $.trim(value).toLowerCase(),
    items = list.children('li.ui-picklist-item'),
    animated = this.isAnimated();

    list.removeAttr('role');

    if(filterValue === '') {
        items.filter(':hidden').show();
        list.attr('role', 'menu');
    }
    else {            
        for(var i = 0; i < items.length; i++) {
            var item = items.eq(i),
            itemLabel = item.attr('data-item-label'),
            matches = this.filterMatcher(itemLabel, filterValue);

            if(matches) {
                var hasRole = list[0].hasAttribute('role');
                if(animated) {
                    item.fadeIn('fast', function() {
                        if(!hasRole) {
                            list.attr('role', 'menu');
                        }
                    });
                }
                else {
                    item.show();
                    if(!hasRole) {
                        list.attr('role', 'menu');
                    }
                }
            }
            else {
                if(animated) {
                    item.fadeOut('fast');
                }
                else {
                    item.hide();
                }
            }
        }
    }

};
     //]]>
</script>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment