dev@javaserverfaces.java.net

Re: Seeking Review: 1760 Ajax special case to not return false;

From: Cagatay Civici <cagatay.civici_at_gmail.com>
Date: Wed, 25 Aug 2010 10:48:00 +0100

Hi Ed,

I think this will fix the issue but as I've understood it relies on server side response to update the visual state on client side, so for @all, @form, @this vetoPreventDefault will be false and "return false;" will be rendered. So when I click on the checkbox
visual state won't be updated instantly and I need to wait until the partial response arrives which will update the checkbox element with new visual state. So locally since things are fast it might not noticable but users might see a delay in visual state update when in a network. For example to test locally I wonder what happens if you put a processing delay of couple seconds in TestBean's getCurrentTimeInMillis.

As an alternative solution, since "return false" is only required to be present in h:commandButton maybe adding a check to the clientBehavior's target component is a UICommand or not can do it. I haven't thought on this entirely so it might not be the best solution.

Thanks,

Cagatay

On Aug 24, 2010, at 8:55 PM, Ed Burns wrote:

> https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1760
>
> Cagatay, if you could please review this, I'd appreciate it.
>
> I'm not convinced this is the best fix, but I wanted to narrow down the
> case where this code would kick in.
>
> Please pay particular attention to my testcase xhtml. In my opinion,
> this *does* show off the problem, and it *is* fixed by my patch.
>
> Special case behavior for ajax return false; generation https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1760
>
>
> SECTION: Modified Files
> ----------------------------
> M jsf-ri/src/main/java/com/sun/faces/renderkit/RenderKitUtils.java
>
> - in getSingleBehaviorHandler(), trap an additional case where the
> ajaxBehavior should not prevent the default action from happening.
>
> A jsf-ri/systest/web/ajax/issue1760NestedAjaxCheckboxRender.xhtml
> M jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java
>
> - testcase fodder
>
>
> SECTION: Diffs
> ----------------------------
>
> Index: jsf-ri/src/main/java/com/sun/faces/renderkit/RenderKitUtils.java
> ===================================================================
> --- jsf-ri/src/main/java/com/sun/faces/renderkit/RenderKitUtils.java (revision 8564)
> +++ jsf-ri/src/main/java/com/sun/faces/renderkit/RenderKitUtils.java (working copy)
> @@ -1585,6 +1585,25 @@
>
> String script = behavior.getScript(bContext);
>
> + // If any one of @all, @this, @form, or the
> + // enclosing component's clientId are in the render list of argument
> + // behavior,
> + boolean vetoPreventDefault = true;
> + if ("click".equals(behaviorEventName) || behavior instanceof AjaxBehavior) {
> + Collection<String> render = ((AjaxBehavior)behavior).getRender();
> + String clientId = component.getClientId(context);
> + for (String cur : render) {
> + if (clientId.equals(cur) ||
> + "@all".equals(cur) ||
> + "@form".equals(cur) ||
> + "@this".equals(cur)) {
> + vetoPreventDefault = false;
> + break;
> + }
> + }
> + }
> +
> +
> // TODO: The "action".equals(behaviorEventName) is a bit awkward - we
> // should find a better solution. The problem that we are trying
> // to avoid is that we do not want to prevent the default behavior
> @@ -1604,7 +1623,7 @@
> preventDefault);
> }
> }
> - else if (preventDefault) {
> + else if (preventDefault && !vetoPreventDefault) {
> script = script + ";return false";
> }
>
> Index: jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java
> ===================================================================
> --- jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java (revision 8564)
> +++ jsf-ri/systest/src/com/sun/faces/systest/model/TestBean.java (working copy)
> @@ -971,4 +971,8 @@
> return result;
> }
>
> + public String getCurrentTimeMillis() {
> + return "" + System.currentTimeMillis();
> + }
> +
> }
> Index: jsf-ri/systest/web/ajax/issue1760NestedAjaxCheckboxRender.xhtml
> ===================================================================
> --- jsf-ri/systest/web/ajax/issue1760NestedAjaxCheckboxRender.xhtml (revision 0)
> +++ jsf-ri/systest/web/ajax/issue1760NestedAjaxCheckboxRender.xhtml (revision 0)
> @@ -0,0 +1,80 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> + <!--
> +
> + DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
> +
> + Copyright 1997-2008 Sun Microsystems, Inc. All rights reserved.
> +
> + The contents of this file are subject to the terms of either the GNU
> + General Public License Version 2 only ("GPL") or the Common Development
> + and Distribution License("CDDL") (collectively, the "License"). You
> + may not use this file except in compliance with the License. You can obtain
> + a copy of the License at https://glassfish.dev.java.net/public/CDDL+GPL.html
> + or glassfish/bootstrap/legal/LICENSE.txt. See the License for the specific
> + language governing permissions and limitations under the License.
> +
> + When distributing the software, include this License Header Notice in each
> + file and include the License file at glassfish/bootstrap/legal/LICENSE.txt.
> + Sun designates this particular file as subject to the "Classpath" exception
> + as provided by Sun in the GPL Version 2 section of the License file that
> + accompanied this code. If applicable, add the following below the License
> + Header, with the fields enclosed by brackets [] replaced by your own
> + identifying information: "Portions Copyrighted [year]
> + [name of copyright owner]"
> +
> + Contributor(s):
> +
> + If you wish your version of this file to be governed by only the CDDL or
> + only the GPL Version 2, indicate your decision by adding "[Contributor]
> + elects to include this software in this distribution under the [CDDL or GPL
> + Version 2] license." If you don't indicate a single choice of license, a
> + recipient has the option to distribute your version of this file under
> + either the CDDL, the GPL Version 2 or to extend the choice of license to
> + its licensees as provided above. However, if you add GPL Version 2 code
> + and therefore, elected the GPL Version 2 license, then the option applies
> + only if the new code is made subject to such option by the copyright
> + holder.
> +
> +-->
> +<!DOCTYPE html
> + PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
> + "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
> +<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"
> + xmlns:h="http://java.sun.com/jsf/html"
> + xmlns:f="http://java.sun.com/jsf/core"
> + xmlns:ui="http://java.sun.com/jsf/facelets">
> +<h:head>
> + <title>Ajax Checkbox rerender</title>
> + <style type="text/css">
> + .grayBox { padding: 8px; margin: 10px 0; border: 1px solid #CCC; background-color: #f9f9f9; }
> + </style>
> +
> +</h:head>
> +<body>
> + <h:form id="form1" prependId="false" styleClass="grayBox">
> +
> + <p>nested ajax with render including component within the same form.</p>
> +
> + <h:outputText id="checkedDisplay" value="#{test1.currentTimeMillis}" />
> +
> + <h:selectBooleanCheckbox value="#{test1.booleanProperty}">
> + <f:ajax render="checkedDisplay" event="click"/>
> + </h:selectBooleanCheckbox>
> +
> + </h:form>
> +
> + <h:form id="form2" styleClass="grayBox">
> +
> + <p>nested ajax with render including @form.</p>
> +
> + <h:outputText id="checkedDisplay2" value="#{test1.currentTimeMillis}" />
> +
> + <h:selectBooleanCheckbox value="#{test1.booleanProperty}">
> + <f:ajax render="@form" event="click"/>
> + </h:selectBooleanCheckbox>
> +
> + </h:form>
> +
> +
> +</body>
> +</html>
>
> --
> | ed.burns_at_sun.com | office: +1 407 458 0017
> | homepage: | http://ridingthecrest.com/
> | 17 work days until JavaOne 2010