ICEfaces
  1. ICEfaces
  2. ICE-7714

h:selectOneMenu update not applied from ValueChangeListener

          Details

          • Type: Bug Bug
          • Status: Closed
          • Priority: Major Major
          • Resolution: Fixed
          • Affects Version/s: 3.0.RC1
          • Fix Version/s: 3.0.1, EE-3.0.0.GA
          • Component/s: Framework
          • Labels:
            None
          • Environment:
            Tomcat 7.0.12
            ICEfaces 3RC1a

            Description

            The valueChangeListener in an h:selectOneMenu is being used to cross validate with another field in one of my applications. If another field is a certain value, the h:selectOneMenu value should be set to '0', even if '0' was the initial value before the change.

            When an attempt is made to set back to '0' the following response comes back in the console:

            <?xml version='1.0' encoding='UTF-8'?>
            <partial-response><changes><update id="javax.faces.ViewState"><![CDATA[574103076592712092:7902382816915448980]]></update><eval><![CDATA[alert('Invalid Cycle');]]></eval><extension aceCallbackParam="validationFailed">{"validationFailed":false}</extension></changes></partial-response>

            When the value is not set to '0', the component updates as expected.

            Attached is a war file and source code for a test case.

              Activity

              Ascending order - Click to sort in descending order
              Brad Kroeger created issue -
              Brad Kroeger made changes -
              Field Original Value New Value
              Attachment Test_war_exploded.war [ 14021 ]
              Hide
              Permalink
              Brad Kroeger added a comment -

              Source code

              Show
              Brad Kroeger added a comment - Source code
              Brad Kroeger made changes -
              Attachment Test.zip [ 14022 ]
              Brad Kroeger made changes -
              Salesforce Case []
              Description The valueChangeListener is an h:selectOneMenu is being used to cross validate with another field in one of my applications. If another field is a certain value, the h:selectOneMenu value should be set to '0', even if '0' was the initial value before the change.

              When an attempt is made to set back to '0' the following response comes back in the console:

              <?xml version='1.0' encoding='UTF-8'?>
              <partial-response><changes><update id="javax.faces.ViewState"><![CDATA[574103076592712092:7902382816915448980]]></update><eval><![CDATA[alert('Invalid Cycle');]]></eval><extension aceCallbackParam="validationFailed">{"validationFailed":false}</extension></changes></partial-response>

              When the value is not set to '0', the component updates as expected.

              Attached is a war file and source code for a test case.               		The valueChangeListener in an h:selectOneMenu is being used to cross validate with another field in one of my applications. If another field is a certain value, the h:selectOneMenu value should be set to '0', even if '0' was the initial value before the change.

              When an attempt is made to set back to '0' the following response comes back in the console:

              <?xml version='1.0' encoding='UTF-8'?>
              <partial-response><changes><update id="javax.faces.ViewState"><![CDATA[574103076592712092:7902382816915448980]]></update><eval><![CDATA[alert('Invalid Cycle');]]></eval><extension aceCallbackParam="validationFailed">{"validationFailed":false}</extension></changes></partial-response>

              When the value is not set to '0', the component updates as expected.

              Attached is a war file and source code for a test case.
              Brad Kroeger made changes -
              Assignee Ken Fyten [ ken.fyten ]
              Ken Fyten made changes -
              Salesforce Case []
              Fix Version/s 3.0.1 [ 10282 ]
              Fix Version/s 3.0 [ 10241 ]
              Assignee Priority P2
              Assignee Ken Fyten [ ken.fyten ] Mark Collette [ mark.collette ]
              Hide
              Permalink
              Mark Collette added a comment -

              Start off with seeing what's going on in:
              org.icefaces.impl.context.DOMPartialViewContext.applyBrowserChanges(Map parameters, Document document)

              Show
              Mark Collette added a comment - Start off with seeing what's going on in: org.icefaces.impl.context.DOMPartialViewContext.applyBrowserChanges(Map parameters, Document document)
              Ken Fyten made changes -
              Salesforce Case []
              Assignee Priority P2 P1
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #27937 Mon Feb 20 10:02:04 MST 2012 mark.collette ICE-7714 : h:selectOneMenu update not applied from ValueChangeListener
              Files Changed
              Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #27938 Mon Feb 20 10:07:30 MST 2012 mark.collette ICE-7714 : h:selectOneMenu update not applied from ValueChangeListener
              Files Changed
              Commit graph MODIFY /icefaces3/branches/icefaces-3.0.x-maintenance/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Ken Fyten made changes -
              Salesforce Case []
              Fix Version/s EE-3.0.0.BETA [ 10324 ]
              Hide
              Permalink
              Mark Collette added a comment -

              The problem was in the method I had thought, but the reason was completely different than I had anticipated. Basically, there was no omission in the select and option node processing code. Rather, for Mojarra, but not MyFaces, the h:selectOneMenu rendering does not follow the standard ResponseWriter APIs of opening and closing tags, which our DOMResponseWriter uses to build DOM nodes. Instead, they have a performance optimisation where they render into a String buffer, and then write that out as text. So what we have in our dom is a Text node, which does not allow for proper applyBrowserChanges processing.

              We contemplated overriding the renderer to allow for proper dom adhering output, but that would have involved overriding a lot of renderer code, that would then need to be maintained. Instead, I thought of a solution whereby we parse the text node, derive the nodes, update the nodes with the typical applyBrowserChanges processing, and then update the actual dom with the new nodes. The idea beig that code path would only execute under very specific circumstances, and wouldn't affect anything else, and would be easy to make a regression test for, since we have the one for this jira.

              One snag I ran into, was I had assumed that I would import the new nodes into our dom, and had that code functioning, but then the dom difference was between nodes and the always rendered text node. So I then printed out the nodes into a string, and updated the text node with that string. There was some effort in getting the whitespace to render properly and similarly.

              There's still a lingering aspect, which I believe we can safely ignore. Our option node updating code outputs W3C compatible markup, which is <option selected="selected"> whereas Mojarra h:selectOneMenu rendering gives <option selected="true">. So in theory we can get false positives in our dom difference, and update more of the page than necessary. But the real bug is failing to update what has changed, so it's not a real problem.

              trunk
              27937

              icefaces-3.0.x-maintenance
              27938

              Show
              Mark Collette added a comment - The problem was in the method I had thought, but the reason was completely different than I had anticipated. Basically, there was no omission in the select and option node processing code. Rather, for Mojarra, but not MyFaces, the h:selectOneMenu rendering does not follow the standard ResponseWriter APIs of opening and closing tags, which our DOMResponseWriter uses to build DOM nodes. Instead, they have a performance optimisation where they render into a String buffer, and then write that out as text. So what we have in our dom is a Text node, which does not allow for proper applyBrowserChanges processing. We contemplated overriding the renderer to allow for proper dom adhering output, but that would have involved overriding a lot of renderer code, that would then need to be maintained. Instead, I thought of a solution whereby we parse the text node, derive the nodes, update the nodes with the typical applyBrowserChanges processing, and then update the actual dom with the new nodes. The idea beig that code path would only execute under very specific circumstances, and wouldn't affect anything else, and would be easy to make a regression test for, since we have the one for this jira. One snag I ran into, was I had assumed that I would import the new nodes into our dom, and had that code functioning, but then the dom difference was between nodes and the always rendered text node. So I then printed out the nodes into a string, and updated the text node with that string. There was some effort in getting the whitespace to render properly and similarly. There's still a lingering aspect, which I believe we can safely ignore. Our option node updating code outputs W3C compatible markup, which is <option selected="selected"> whereas Mojarra h:selectOneMenu rendering gives <option selected="true">. So in theory we can get false positives in our dom difference, and update more of the page than necessary. But the real bug is failing to update what has changed, so it's not a real problem. trunk 27937 icefaces-3.0.x-maintenance 27938
              Mark Collette made changes -
              Status Open [ 1 ] Resolved [ 5 ]
              Fix Version/s EE-3.0.0.BETA [ 10324 ]
              Resolution Fixed [ 1 ]
              Ken Fyten made changes -
              Salesforce Case []
              Fix Version/s EE-3.0.0.BETA [ 10324 ]
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #27956 Tue Feb 21 13:49:22 MST 2012 ted.goddard removed System.out (ICE-7714)
              Files Changed
              Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #27957 Tue Feb 21 14:27:56 MST 2012 ted.goddard removed System.out (ICE-7714)
              Files Changed
              Commit graph MODIFY /icefaces3/branches/icefaces-3.0.x-maintenance/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #27980 Wed Feb 22 13:00:38 MST 2012 ken.fyten Reverted ICE-7714 commit due to regressions.
              Files Changed
              Commit graph MODIFY /icefaces3/branches/icefaces-3.0.x-maintenance/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Hide
              Permalink
              Ken Fyten added a comment - - edited

              Re-opened due to regressions.

              Arran McCullough: Throws SAXParseExceptions with customer application.

              All browsers and this doctype: <!DOCTYPE HTML PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">

              I think it might be related to them using & in the selectItem label.

              — Ken Fyten:

              I've reverted this commit on ossrepo/icefaces3/branches/icefaces-3.0.x-maintenance branch for EE 3 Beta release.

              Show
              Ken Fyten added a comment - - edited Re-opened due to regressions. Arran McCullough: Throws SAXParseExceptions with customer application. All browsers and this doctype: <!DOCTYPE HTML PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> I think it might be related to them using & in the selectItem label. — Ken Fyten: I've reverted this commit on ossrepo/icefaces3/branches/icefaces-3.0.x-maintenance branch for EE 3 Beta release.
              Ken Fyten made changes -
              Resolution Fixed [ 1 ]
              Status Resolved [ 5 ] Reopened [ 4 ]
              Ken Fyten made changes -
              Salesforce Case []
              Fix Version/s EE-3.0.0.GA [ 10262 ]
              Fix Version/s EE-3.0.0.BETA [ 10324 ]
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #27993 Wed Feb 22 16:32:19 MST 2012 ken.fyten Reverted ICE-7714 commit due to regressions.
              Files Changed
              Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Hide
              Permalink
              Ken Fyten added a comment -

              Also reverted on icefaces3/trunk: Committed revision 27993

              Show
              Ken Fyten added a comment - Also reverted on icefaces3/trunk: Committed revision 27993
              Hide
              Permalink
              Ken Fyten added a comment -

              Ideas to resolve:

              • Use HTML DOM if available in JDK.
              • Experiment with DOCtypes for DOM
              • Add entity characters to DOM
              • Use regular expression or other technique to replace entity chars in the string.
              Show
              Ken Fyten added a comment - Ideas to resolve: Use HTML DOM if available in JDK. Experiment with DOCtypes for DOM Add entity characters to DOM Use regular expression or other technique to replace entity chars in the string.
              Ken Fyten made changes -
              Assignee Mark Collette [ mark.collette ] Ted Goddard [ ted.goddard ]
              Hide
              Permalink
              Arran Mccullough added a comment -

              Can see the issue with the following code:

              <h:selectOneMenu id="testMenu"
              value="#

              {testBean.menuValue}

              ">
              <f:selectItem itemValue="1" itemLabel="One & One"/>
              <f:selectItem itemValue="2" itemLabel="Two and Two"/>
              </h:selectOneMenu>

              Show
              Arran Mccullough added a comment - Can see the issue with the following code: <h:selectOneMenu id="testMenu" value="# {testBean.menuValue} "> <f:selectItem itemValue="1" itemLabel="One & One"/> <f:selectItem itemValue="2" itemLabel="Two and Two"/> </h:selectOneMenu>
              Hide
              Permalink
              Ted Goddard added a comment -

              Since the DOM parser version changes the order of value="" and selected="", it is essentially guaranteed to modify the TextNode for any interaction with the component, thereby forcing an update of that component.

              Show
              Ted Goddard added a comment - Since the DOM parser version changes the order of value="" and selected="", it is essentially guaranteed to modify the TextNode for any interaction with the component, thereby forcing an update of that component.
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #28020 Fri Feb 24 14:58:52 MST 2012 ted.goddard using regex to make minimal modifications to h:selectOneMenu output during applyBrowserChanges (ICE-7714)
              Files Changed
              Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #28021 Fri Feb 24 15:01:13 MST 2012 ted.goddard using regex to make minimal modifications to
              h:selectOneMenu output during applyBrowserChanges (ICE-7714)
              Files Changed
              Commit graph MODIFY /icefaces3/branches/icefaces-3.0.x-maintenance/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
              Ted Goddard made changes -
              Attachment Test2.zip [ 14094 ]
              Hide
              Permalink
              Ted Goddard added a comment -

              Modified Test2.zip to include ampersand markup as well as changed the test logic to accept a selection of "3" (this allows the testing of changed input values that should not result in a page update).

              Show
              Ted Goddard added a comment - Modified Test2.zip to include ampersand markup as well as changed the test logic to accept a selection of "3" (this allows the testing of changed input values that should not result in a page update).
              Hide
              Permalink
              Ted Goddard added a comment -

              Regex implementation checked in but not verified with multiple select cases. The previous intermediate-DOM based version could not be repaired due to the output differences between the original component and the intermediate DOM output.

              Show
              Ted Goddard added a comment - Regex implementation checked in but not verified with multiple select cases. The previous intermediate-DOM based version could not be repaired due to the output differences between the original component and the intermediate DOM output.
              Ted Goddard made changes -
              Status Reopened [ 4 ] Resolved [ 5 ]
              Resolution Fixed [ 1 ]
              Hide
              Permalink
              Ted Goddard added a comment -

              The encodeEnd() method of MenuRenderer which makes use of FastStringWriter to embed the text node in the DOM does allow for a "multiple" select option.

              Show
              Ted Goddard added a comment - The encodeEnd() method of MenuRenderer which makes use of FastStringWriter to embed the text node in the DOM does allow for a "multiple" select option.
              Ken Fyten made changes -
              Status Resolved [ 5 ] Closed [ 6 ]
              Assignee Priority P1

                People

                • Assignee:
                  Ted Goddard
                  Reporter:
                  Brad Kroeger
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  1 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: