javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-2283

System Events Example App: Dynamic Components

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Works as designed
    • Affects Version/s: 2.0.7, 2.1.6
    • Fix Version/s: 2.1.9, 2.2.0-m03
    • Component/s: None
    • Labels:
      None

      Description

      When using partial state saving, dynamic UISelectItem / UISelectItems are not restored upon postback. We need to investigate if UISelectItem and UISelectItems should be added to the list of dynamic actions in the first place. Note get the test application from issue 1826, the addressbook-faces2.war attachment.

        Issue Links

          Activity

          Hide
          kennardconsulting added a comment -

          With respect to the latest JAR (javax.faces-2.1.7-20120112.181559-11.jar), this issue now appears to work in some cases. But not in others:

          1. Go to http://localhost:8080/addressbook-faces2
          2. Click 'Homer Simpson'
          3. Click 'Edit'
          4. Click 'Save'

          Note the Gender UISelectItem has lost its values. Also you will see the field ordering has changed significantly.

          Show
          kennardconsulting added a comment - With respect to the latest JAR (javax.faces-2.1.7-20120112.181559-11.jar), this issue now appears to work in some cases. But not in others: 1. Go to http://localhost:8080/addressbook-faces2 2. Click 'Homer Simpson' 3. Click 'Edit' 4. Click 'Save' Note the Gender UISelectItem has lost its values. Also you will see the field ordering has changed significantly.
          Hide
          kennardconsulting added a comment -

          Manfred,

          If you must close JAVASERVERFACES-1826 because you "wanted to make sure that these changes would get into the code base as there are a number of other issues dependent on this issue", can you at least mark issue JAVASERVERFACES-2283 as 'major' priority (just like 1826 was?)

          I must stress that the issue as a whole (i.e. that dynamic component manipulation works in Mojarra) is not resolved. And it goes beyond just UISelectItems.

          Regards,

          Richard.

          Show
          kennardconsulting added a comment - Manfred, If you must close JAVASERVERFACES-1826 because you "wanted to make sure that these changes would get into the code base as there are a number of other issues dependent on this issue", can you at least mark issue JAVASERVERFACES-2283 as 'major' priority (just like 1826 was?) I must stress that the issue as a whole (i.e. that dynamic component manipulation works in Mojarra) is not resolved. And it goes beyond just UISelectItems. Regards, Richard.
          Hide
          kennardconsulting added a comment - - edited

          The overarching problem is that partial state saving in Mojarra is broken for dynamic components. I've been whinging about this for many months across many issues (JAVASERVERFACES-1383, JAVASERVERFACES-1402, JAVASERVERFACES-1313, JAVASERVERFACES-1717). Others have noticed it too (JAVASERVERFACES-2231, JAVASERVERFACES-1414, JAVASERVERFACES-1972). It's a complicated issue with many symptoms and side effects. JAVASERVERFACES-1826 was my attempt to help you guys by providing a number of test cases.

          However issue 1826 could never hope to cover all edge cases of your particular implementation

          What has happened is that you've finished JAVASERVERFACES-1826 and closed it, and spun off one of the remaining issues into JAVASERVERFACES-2283. This new issue is marked minor. But it is not minor! It is still blocking anything from working reliably.

          The issue of dynamic components in Mojarra is not solved

          Could you please make sure this is understood? At the very least, I would want to see JAVASERVERFACES-2283 given a priority of 'high' (same as JAVASERVERFACES-1826 used to be). I think it should be renamed too. What has happened is we've moved on from 'here are a bunch of little acid tests, please make sure they work' to 'here is a small web app, please make sure it works'. And the small web app does not work. It has a number of problems, from missing UISelectItems to scrambled component ordering, which are not present under MyFaces.

          I'm worried we are going to lose momentum. Even though much has been accomplished, all of it is for nothing unless everything works.

          Show
          kennardconsulting added a comment - - edited The overarching problem is that partial state saving in Mojarra is broken for dynamic components. I've been whinging about this for many months across many issues ( JAVASERVERFACES-1383 , JAVASERVERFACES-1402 , JAVASERVERFACES-1313 , JAVASERVERFACES-1717 ). Others have noticed it too ( JAVASERVERFACES-2231 , JAVASERVERFACES-1414 , JAVASERVERFACES-1972 ). It's a complicated issue with many symptoms and side effects. JAVASERVERFACES-1826 was my attempt to help you guys by providing a number of test cases. However issue 1826 could never hope to cover all edge cases of your particular implementation What has happened is that you've finished JAVASERVERFACES-1826 and closed it, and spun off one of the remaining issues into JAVASERVERFACES-2283 . This new issue is marked minor. But it is not minor! It is still blocking anything from working reliably. The issue of dynamic components in Mojarra is not solved Could you please make sure this is understood? At the very least, I would want to see JAVASERVERFACES-2283 given a priority of 'high' (same as JAVASERVERFACES-1826 used to be). I think it should be renamed too. What has happened is we've moved on from 'here are a bunch of little acid tests, please make sure they work' to 'here is a small web app, please make sure it works'. And the small web app does not work. It has a number of problems, from missing UISelectItems to scrambled component ordering, which are not present under MyFaces. I'm worried we are going to lose momentum. Even though much has been accomplished, all of it is for nothing unless everything works.
          Hide
          rogerk added a comment -

          Change title and priority.

          Show
          rogerk added a comment - Change title and priority.
          Hide
          kennardconsulting added a comment -

          Hi guys,

          Has anybody had chance to look at this issue? Maybe to at least confirm they can reproduce it?

          Regards,

          Richard.

          Show
          kennardconsulting added a comment - Hi guys, Has anybody had chance to look at this issue? Maybe to at least confirm they can reproduce it? Regards, Richard.
          Hide
          ova2 added a comment -

          I agree, the issue with dynamic components is not solved. See also my issue http://java.net/jira/browse/JAVASERVERFACES-2383

          Show
          ova2 added a comment - I agree, the issue with dynamic components is not solved. See also my issue http://java.net/jira/browse/JAVASERVERFACES-2383
          Hide
          kennardconsulting added a comment -

          Can someone please comment on the timeline for this issue?

          Show
          kennardconsulting added a comment - Can someone please comment on the timeline for this issue?
          Hide
          kennardconsulting added a comment - - edited

          Buoyed by the progress on JAVASERVERFACES-1414, I have revisited and updated the test application for this issue (attached).

          Still no luck with javax.faces-2.1.9-20120524.180105-1.jar or javax.faces-2.2.0-20120518.034337-98.jar I'm afraid. Works fine with MyFaces.

          Show
          kennardconsulting added a comment - - edited Buoyed by the progress on JAVASERVERFACES-1414 , I have revisited and updated the test application for this issue (attached). Still no luck with javax.faces-2.1.9-20120524.180105-1.jar or javax.faces-2.2.0-20120518.034337-98.jar I'm afraid. Works fine with MyFaces.
          Hide
          Manfred Riem added a comment -

          I tried to use the attached project, but I can't seem to build it. It appears a dependency is missing?

          Show
          Manfred Riem added a comment - I tried to use the attached project, but I can't seem to build it. It appears a dependency is missing?
          Hide
          kennardconsulting added a comment -

          Hi Manfred,

          I just tried this again and it seems okay to me? I...

          1. Downloaded and unzipped the attached addressbook-faces2.zip
          2. Opened Eclipse (with m2e installed)
          3. Used Import... Existing Maven Projects
          4. Published to Tomcat 7.0.25

          ...and it ran okay? What dependency are you seeing missing?

          Note the attachment includes libraries in a WEB-INF/lib. It also includes MyFaces (so that you can see it working). You will need to manually swap in Mojarra.

          Regards,

          Richard.

          Show
          kennardconsulting added a comment - Hi Manfred, I just tried this again and it seems okay to me? I... 1. Downloaded and unzipped the attached addressbook-faces2.zip 2. Opened Eclipse (with m2e installed) 3. Used Import... Existing Maven Projects 4. Published to Tomcat 7.0.25 ...and it ran okay? What dependency are you seeing missing? Note the attachment includes libraries in a WEB-INF/lib. It also includes MyFaces (so that you can see it working). You will need to manually swap in Mojarra. Regards, Richard.
          Hide
          Manfred Riem added a comment - - edited

          This is what I get:

          [compiler:compile]
          -------------------------------------------------------------
          COMPILATION ERROR :
          -------------------------------------------------------------
          org/metawidget/example/shared/addressbook/util/ExampleUtils.java:[23,26] error: package org.metawidget.util does not exist
          org/metawidget/example/shared/addressbook/model/Contact.java:[19,38] error: package org.metawidget.inspector does not exist
          org/metawidget/example/shared/addressbook/model/Contact.java:[26,42] error: package org.metawidget.inspector.annotation does not exist

          Show
          Manfred Riem added a comment - - edited This is what I get: [compiler:compile] ------------------------------------------------------------- COMPILATION ERROR : ------------------------------------------------------------- org/metawidget/example/shared/addressbook/util/ExampleUtils.java: [23,26] error: package org.metawidget.util does not exist org/metawidget/example/shared/addressbook/model/Contact.java: [19,38] error: package org.metawidget.inspector does not exist org/metawidget/example/shared/addressbook/model/Contact.java: [26,42] error: package org.metawidget.inspector.annotation does not exist
          Hide
          kennardconsulting added a comment -

          Hi Manfred,

          Thanks for trying to reproduce this issue. Sorry for the confusion!

          The JAR you are missing is located inside the src/main/webapp/WEB-INF/lib of the ZIP. So you need that folder on your CLASSPATH. If you use Eclipse (with m2e installed) this all happens automatically. You can set up a server in the Servers tab and deploy the project to it automatically. You won't have to run Maven directly.

          I did it this way (rather than putting all dependencies in the pom.xml) because I had to modify metawidget-all.jar slightly. The normal metawidget-all.jar has an explicit check to warn users and stop it running with Mojarra (until bug 2283 is fixed).

          Regards,

          Richard.

          Show
          kennardconsulting added a comment - Hi Manfred, Thanks for trying to reproduce this issue. Sorry for the confusion! The JAR you are missing is located inside the src/main/webapp/WEB-INF/lib of the ZIP. So you need that folder on your CLASSPATH. If you use Eclipse (with m2e installed) this all happens automatically. You can set up a server in the Servers tab and deploy the project to it automatically. You won't have to run Maven directly. I did it this way (rather than putting all dependencies in the pom.xml) because I had to modify metawidget-all.jar slightly. The normal metawidget-all.jar has an explicit check to warn users and stop it running with Mojarra (until bug 2283 is fixed). Regards, Richard.
          Hide
          Manfred Riem added a comment -

          Is it possible to remove those dependencies? I want to make sure we can integrate this test into our test harness so we don't experience any regressions on this!

          Show
          Manfred Riem added a comment - Is it possible to remove those dependencies? I want to make sure we can integrate this test into our test harness so we don't experience any regressions on this!
          Hide
          kennardconsulting added a comment -

          Manfred,

          Apologies for the confusion.

          I did go through an exercise removing dependencies and simulating scenarios in order to produce regression tests. I simulated 8 scenarios that way (see JAVASERVERFACES-1826). The problem is my simulated scenarios cannot anticipate all implementation details of your particular Mojarra implementation (they were sufficient for the MyFaces team).

          So the next logical step is to give you the small app itself so you can determine what other cases are failing. Note the small app works in the same way as the JAVASERVERFACES-1826 scenarios, and works fine under MyFaces.

          I'm hopeful you can use this app to see what further regression tests you need to simulate.

          Regards,

          Richard.

          Show
          kennardconsulting added a comment - Manfred, Apologies for the confusion. I did go through an exercise removing dependencies and simulating scenarios in order to produce regression tests. I simulated 8 scenarios that way (see JAVASERVERFACES-1826 ). The problem is my simulated scenarios cannot anticipate all implementation details of your particular Mojarra implementation (they were sufficient for the MyFaces team). So the next logical step is to give you the small app itself so you can determine what other cases are failing. Note the small app works in the same way as the JAVASERVERFACES-1826 scenarios, and works fine under MyFaces. I'm hopeful you can use this app to see what further regression tests you need to simulate. Regards, Richard.
          Hide
          Manfred Riem added a comment -

          After working to make it compilable I now get the following stacktrace when I run your application:

          javax.faces.FacesException: Cannot add the same component twice: form:j_id3
          at com.sun.faces.context.StateContext$AddRemoveListener.handleAddRemoveWithAutoPrune(StateContext.java:476)
          at com.sun.faces.context.StateContext$AddRemoveListener.handleAdd(StateContext.java:422)
          at com.sun.faces.context.StateContext$AddRemoveListener.processEvent(StateContext.java:338)
          at javax.faces.event.SystemEvent.processListener(SystemEvent.java:106)
          at com.sun.faces.application.ApplicationImpl.processListenersAccountingForAdds(ApplicationImpl.java:2217)
          at com.sun.faces.application.ApplicationImpl.invokeViewListenersFor(ApplicationImpl.java:2038)
          at com.sun.faces.application.ApplicationImpl.publishEvent(ApplicationImpl.java:291)
          at com.sun.faces.application.ApplicationImpl.publishEvent(ApplicationImpl.java:246)
          at javax.faces.component.UIComponentBase.publishAfterViewEvents(UIComponentBase.java:2201)
          at javax.faces.component.UIComponentBase.doPostAddProcessing(UIComponentBase.java:1883)
          at javax.faces.component.UIComponentBase.setParent(UIComponentBase.java:400)
          at org.metawidget.faces.renderkit.html.HtmlLayoutRenderer.createLabel(HtmlLayoutRenderer.java:175)
          at org.metawidget.faces.renderkit.html.HtmlLayoutRenderer.layoutLabel(HtmlLayoutRenderer.java:147)
          at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.layoutLabel(HtmlTableLayoutRenderer.java:456)
          at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.layoutBeforeChild(HtmlTableLayoutRenderer.java:355)
          at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.encodeChildren(HtmlTableLayoutRenderer.java:288)
          at javax.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:845)
          at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779)
          at javax.faces.render.Renderer.encodeChildren(Renderer.java:168)
          at javax.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:845)
          at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779)
          at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1782)
          at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1782)
          at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:424)
          at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:125)
          at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:121)
          at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101)
          at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:139)
          at javax.faces.webapp.FacesServlet.service(FacesServlet.java:594)

          Show
          Manfred Riem added a comment - After working to make it compilable I now get the following stacktrace when I run your application: javax.faces.FacesException: Cannot add the same component twice: form:j_id3 at com.sun.faces.context.StateContext$AddRemoveListener.handleAddRemoveWithAutoPrune(StateContext.java:476) at com.sun.faces.context.StateContext$AddRemoveListener.handleAdd(StateContext.java:422) at com.sun.faces.context.StateContext$AddRemoveListener.processEvent(StateContext.java:338) at javax.faces.event.SystemEvent.processListener(SystemEvent.java:106) at com.sun.faces.application.ApplicationImpl.processListenersAccountingForAdds(ApplicationImpl.java:2217) at com.sun.faces.application.ApplicationImpl.invokeViewListenersFor(ApplicationImpl.java:2038) at com.sun.faces.application.ApplicationImpl.publishEvent(ApplicationImpl.java:291) at com.sun.faces.application.ApplicationImpl.publishEvent(ApplicationImpl.java:246) at javax.faces.component.UIComponentBase.publishAfterViewEvents(UIComponentBase.java:2201) at javax.faces.component.UIComponentBase.doPostAddProcessing(UIComponentBase.java:1883) at javax.faces.component.UIComponentBase.setParent(UIComponentBase.java:400) at org.metawidget.faces.renderkit.html.HtmlLayoutRenderer.createLabel(HtmlLayoutRenderer.java:175) at org.metawidget.faces.renderkit.html.HtmlLayoutRenderer.layoutLabel(HtmlLayoutRenderer.java:147) at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.layoutLabel(HtmlTableLayoutRenderer.java:456) at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.layoutBeforeChild(HtmlTableLayoutRenderer.java:355) at org.metawidget.faces.renderkit.html.HtmlTableLayoutRenderer.encodeChildren(HtmlTableLayoutRenderer.java:288) at javax.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:845) at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779) at javax.faces.render.Renderer.encodeChildren(Renderer.java:168) at javax.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:845) at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779) at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1782) at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1782) at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:424) at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:125) at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:121) at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101) at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:139) at javax.faces.webapp.FacesServlet.service(FacesServlet.java:594)
          Hide
          kennardconsulting added a comment - - edited

          Manfred,

          Thanks for taking the time to get the test working. I know this isn't your preferred approach! However the advantage is we no longer have a 'moving target' for our test case. This is important because the errors I see vary from implementation to implementation. For example:

          MyFaces 2.0.7: works, no problems
          Mojarra 2.1.9-20120524.180105-1: gets the 'Cannot add the same component twice: form:j_id3' error you describe
          Mojarra 2.2.0-20120518.034337-98: almost works, but reshuffles field ordering (as described in steps 1-4 at top of this JIRA)

          I hope this helps you debug it.

          Regards,

          Richard.

          Show
          kennardconsulting added a comment - - edited Manfred, Thanks for taking the time to get the test working. I know this isn't your preferred approach! However the advantage is we no longer have a 'moving target' for our test case. This is important because the errors I see vary from implementation to implementation. For example: MyFaces 2.0.7: works, no problems Mojarra 2.1.9-20120524.180105-1: gets the 'Cannot add the same component twice: form:j_id3' error you describe Mojarra 2.2.0-20120518.034337-98: almost works, but reshuffles field ordering (as described in steps 1-4 at top of this JIRA) I hope this helps you debug it. Regards, Richard.
          Hide
          Manfred Riem added a comment -

          OK these are the results after testing each version. Note that version 2.1.9 and upwards, and 2.2.0-m03 and upwards all signal that your code is trying to add the same component twice. The listener specifically guards against that since allowing it would violate the JSF specification.

          Results per version
          2.1.6 - search throws NullPointerException
          2.1.7 - search shows not a valid option
          2.1.8 - search shows not a valid option
          2.1.9 - cannot add component twice
          2.1.10 - cannot add component twice
          2.1.11 - cannot add component twice
          2.1.12-20120719.084854-6 - cannot add component twice

          2.2.0-m01 - throws IllegalStateException
          2.2.0-m02 - search throws NullPointerException
          2.2.0-m03 - cannot add component twice
          2.2.0-m04 - cannot add component twice
          2.2.0-20120719.080026-131 - cannot add component twice

          Can you investigate in your code how/why it is adding a component twice?

          Show
          Manfred Riem added a comment - OK these are the results after testing each version. Note that version 2.1.9 and upwards, and 2.2.0-m03 and upwards all signal that your code is trying to add the same component twice. The listener specifically guards against that since allowing it would violate the JSF specification. Results per version 2.1.6 - search throws NullPointerException 2.1.7 - search shows not a valid option 2.1.8 - search shows not a valid option 2.1.9 - cannot add component twice 2.1.10 - cannot add component twice 2.1.11 - cannot add component twice 2.1.12-20120719.084854-6 - cannot add component twice 2.2.0-m01 - throws IllegalStateException 2.2.0-m02 - search throws NullPointerException 2.2.0-m03 - cannot add component twice 2.2.0-m04 - cannot add component twice 2.2.0-20120719.080026-131 - cannot add component twice Can you investigate in your code how/why it is adding a component twice?
          Hide
          kennardconsulting added a comment - - edited

          Manfred,

          Thanks again for your time on this!

          I don't believe it is adding a component twice. I would base this on:

          a) works fine in MyFaces 2.0.3 and above
          b) works in Mojarra 2.2.0-20120518.034337-98.jar, apart form some re-ordering problems
          c) we've seen 'duplicate component' error before when doing JAVASERVERFACES-1826, and it was invariably a bug in the ViewState code.

          However, I'm very used to being wrong! So I've created a new version of metawidget-all.jar (attached) that outputs some System.out's for you. You'll see that the offending j_id3 is actually a UISelectItem:

          Adding javax.faces.component.html.HtmlInputText@203ba002 with value binding #

          {contactSearch.current.firstname}

          and id contactSearchCurrentFirstname
          Adding javax.faces.component.html.HtmlInputText@777490ad with value binding #

          {contactSearch.current.surname}

          and id contactSearchCurrentSurname
          Adding javax.faces.component.UISelectItem@3cdc904a with id j_id2
          Adding javax.faces.component.UISelectItem@a32087b with id j_id3
          Adding javax.faces.component.UISelectItem@5acac877 with id j_id4
          Adding javax.faces.component.html.HtmlSelectOneMenu@4271c5bc with value binding #

          {contactSearch.current.type}

          and id contactSearchCurrentType
          Adding org.metawidget.faces.component.UIStub@ab245dc with value binding #

          {contactSearch.current}

          and id j_id5
          Adding org.metawidget.faces.component.UIStub@3e087400 with value binding #

          {contactSearch.results}

          and id j_id6

          It's not a 'duplicate' as such (it has a different id, and different itemValue) but there are multiple UISelectItems. Given UISelectItems has been a problem area, perhaps this is a clue?

          Show
          kennardconsulting added a comment - - edited Manfred, Thanks again for your time on this! I don't believe it is adding a component twice. I would base this on: a) works fine in MyFaces 2.0.3 and above b) works in Mojarra 2.2.0-20120518.034337-98.jar, apart form some re-ordering problems c) we've seen 'duplicate component' error before when doing JAVASERVERFACES-1826 , and it was invariably a bug in the ViewState code. However, I'm very used to being wrong! So I've created a new version of metawidget-all.jar (attached) that outputs some System.out's for you. You'll see that the offending j_id3 is actually a UISelectItem: Adding javax.faces.component.html.HtmlInputText@203ba002 with value binding # {contactSearch.current.firstname} and id contactSearchCurrentFirstname Adding javax.faces.component.html.HtmlInputText@777490ad with value binding # {contactSearch.current.surname} and id contactSearchCurrentSurname Adding javax.faces.component.UISelectItem@3cdc904a with id j_id2 Adding javax.faces.component.UISelectItem@a32087b with id j_id3 Adding javax.faces.component.UISelectItem@5acac877 with id j_id4 Adding javax.faces.component.html.HtmlSelectOneMenu@4271c5bc with value binding # {contactSearch.current.type} and id contactSearchCurrentType Adding org.metawidget.faces.component.UIStub@ab245dc with value binding # {contactSearch.current} and id j_id5 Adding org.metawidget.faces.component.UIStub@3e087400 with value binding # {contactSearch.results} and id j_id6 It's not a 'duplicate' as such (it has a different id, and different itemValue) but there are multiple UISelectItems. Given UISelectItems has been a problem area, perhaps this is a clue?
          Hide
          kennardconsulting added a comment - - edited

          Version of Metawidget with extra debug

          Show
          kennardconsulting added a comment - - edited Version of Metawidget with extra debug
          Hide
          Manfred Riem added a comment - - edited

          Debugging through it now (thanks for telling me what the component type was) shows that the UISelectItem has the same id set as the label it is trying to add. I believe the problem stems from this block of code:

          private void addSelectItem( UIComponent component, Object value, String label, UIMetawidget metawidget ) {

          FacesContext context = FacesContext.getCurrentInstance();
          Application application = context.getApplication();

          UISelectItem selectItem = (UISelectItem) application.createComponent( UISelectItem.COMPONENT_TYPE );
          selectItem.setId( context.getViewRoot().createUniqueId() );

          It looks like you are assigning an id to the UISelectItem when you add it to the component tree. However you are asking the UIViewRoot for a unique id, whereas you should be asking the closest UINamingContainer for a unique id (which in this case is UIForm).

          Can you please change that and try the application again?

          Show
          Manfred Riem added a comment - - edited Debugging through it now (thanks for telling me what the component type was) shows that the UISelectItem has the same id set as the label it is trying to add. I believe the problem stems from this block of code: private void addSelectItem( UIComponent component, Object value, String label, UIMetawidget metawidget ) { FacesContext context = FacesContext.getCurrentInstance(); Application application = context.getApplication(); UISelectItem selectItem = (UISelectItem) application.createComponent( UISelectItem.COMPONENT_TYPE ); selectItem.setId( context.getViewRoot().createUniqueId() ); It looks like you are assigning an id to the UISelectItem when you add it to the component tree. However you are asking the UIViewRoot for a unique id, whereas you should be asking the closest UINamingContainer for a unique id (which in this case is UIForm). Can you please change that and try the application again?
          Hide
          ova2 added a comment -

          Hi Manfred,

          "It looks like you are assigning an id to the UISelectItem when you add it to the component tree. However you are asking the UIViewRoot for a unique id, whereas you should be asking the closest UINamingContainer for a unique id".

          Really? I'm a component developer too and I always ask the UIViewRoot for an unique Id. Never had problems with that. Believe me, I did this many times and everything is fine until now for both impls. - Mojarra and MyFaces. I think context.getViewRoot().createUniqueId() is a quite proper approach. I did the same e.g. for UIParameter and UISelectItems (never tried UISelectItem, but UISelectItems is ok). Just one problem - I couldn't create nested components programmatically (issue about non unique Ids). For instance, if you try to create nested components (component + its child) in a TagHandler inside public void onComponentPopulated (...), you will face a "non unique Ids" exception. The best approach is to append some string to the original Id. Something like

          public void onComponentPopulated(FaceletContext ctx, UIComponent c, UIComponent parent) {
          if (!isNew(parent))

          { return; }

          Application app = ctx.getFacesContext().getApplication();
          RefObjectTypeInput refObjectTypeInput = (RefObjectTypeInput) c;
          String id = refObjectTypeInput.getId();

          // label
          HtmlOutputText label = (HtmlOutputText) app.createComponent(HtmlOutputText.COMPONENT_TYPE);
          label.setId(id + "_label");
          ...
          refObjectTypeInput.getChildren().add(label);
          }

          This works smoothly.

          Show
          ova2 added a comment - Hi Manfred, "It looks like you are assigning an id to the UISelectItem when you add it to the component tree. However you are asking the UIViewRoot for a unique id, whereas you should be asking the closest UINamingContainer for a unique id". Really? I'm a component developer too and I always ask the UIViewRoot for an unique Id. Never had problems with that. Believe me, I did this many times and everything is fine until now for both impls. - Mojarra and MyFaces. I think context.getViewRoot().createUniqueId() is a quite proper approach. I did the same e.g. for UIParameter and UISelectItems (never tried UISelectItem, but UISelectItems is ok). Just one problem - I couldn't create nested components programmatically (issue about non unique Ids). For instance, if you try to create nested components (component + its child) in a TagHandler inside public void onComponentPopulated (...), you will face a "non unique Ids" exception. The best approach is to append some string to the original Id. Something like public void onComponentPopulated(FaceletContext ctx, UIComponent c, UIComponent parent) { if (!isNew(parent)) { return; } Application app = ctx.getFacesContext().getApplication(); RefObjectTypeInput refObjectTypeInput = (RefObjectTypeInput) c; String id = refObjectTypeInput.getId(); // label HtmlOutputText label = (HtmlOutputText) app.createComponent(HtmlOutputText.COMPONENT_TYPE); label.setId(id + "_label"); ... refObjectTypeInput.getChildren().add(label); } This works smoothly.
          Hide
          kennardconsulting added a comment - - edited

          Manfred,

          Thanks so much for diving into the Metawidget source code! I really appreciate you going the extra mile. I tried your suggestion and am delighted to report it makes an enormous difference. I'm still running tests but so far everything looks perfect!

          However I agree, Ova2, this is surprising advice. It appears that being able to call createUniqueId from something other than UIViewRoot is new to JSF2? It's part of the new UniqueIdVendor interface?

          Isn't this, therefore, a change in the spec that causes an incompatibility between JSF1 and JSF2? Because you're saying that whilst in JSF1 UIViewRoot.createUniqueId was sufficient for uniqueness, in JSF2 it is not?

          Manfred: Can you please double-check UIViewRoot.createUniqueId shouldn't remain 'completely, heavyweight unique' whilst the new UniqueIdVendor offers some kind of 'local, more efficient, uniqueness'? The JavaDoc for UniqueIdVendor says "reduce the amount of id generation variance... helpful for improved state saving". The phrasing ('reduce', 'helpful', 'improved') seems to imply it is just an optimization rather than a requirement?

          Also, the JSF 2.0 spec only ever talks about UIViewRoot.createUniqueId as far as I can see?

          Show
          kennardconsulting added a comment - - edited Manfred, Thanks so much for diving into the Metawidget source code! I really appreciate you going the extra mile. I tried your suggestion and am delighted to report it makes an enormous difference. I'm still running tests but so far everything looks perfect! However I agree, Ova2, this is surprising advice. It appears that being able to call createUniqueId from something other than UIViewRoot is new to JSF2? It's part of the new UniqueIdVendor interface? Isn't this, therefore, a change in the spec that causes an incompatibility between JSF1 and JSF2? Because you're saying that whilst in JSF1 UIViewRoot.createUniqueId was sufficient for uniqueness, in JSF2 it is not? Manfred: Can you please double-check UIViewRoot.createUniqueId shouldn't remain 'completely, heavyweight unique' whilst the new UniqueIdVendor offers some kind of 'local, more efficient, uniqueness'? The JavaDoc for UniqueIdVendor says "reduce the amount of id generation variance... helpful for improved state saving". The phrasing ('reduce', 'helpful', 'improved') seems to imply it is just an optimization rather than a requirement? Also, the JSF 2.0 spec only ever talks about UIViewRoot.createUniqueId as far as I can see?
          Hide
          kennardconsulting added a comment - - edited

          Manfred,

          After further testing, it appears there are some points of contention between how MyFaces and Mojarra handle this. There seem to be cases, when using MyFaces 2 with RichFaces Ajax processing, that UIForm.createUniqueId will just return 'j_id3' (or whatever) without also incrementing the global value returned by UIViewRoot.createUniqueId. This leads to duplicate id conflicts.

          So I guess one of the teams has an incorrect implementation? Before I harrass the MyFaces guys, can you please double-check the spec permits UIViewRoot.createUniqueId to create non-unique Ids (in cases where there is a nested component that implements UniqueIdVendor)?

          Show
          kennardconsulting added a comment - - edited Manfred, After further testing, it appears there are some points of contention between how MyFaces and Mojarra handle this. There seem to be cases, when using MyFaces 2 with RichFaces Ajax processing, that UIForm.createUniqueId will just return 'j_id3' (or whatever) without also incrementing the global value returned by UIViewRoot.createUniqueId. This leads to duplicate id conflicts. So I guess one of the teams has an incorrect implementation? Before I harrass the MyFaces guys, can you please double-check the spec permits UIViewRoot.createUniqueId to create non-unique Ids (in cases where there is a nested component that implements UniqueIdVendor)?
          Hide
          Ed Burns added a comment -

          Hello Richard and Manfred,

          Thanks to you both for helping to make JSF more robust. While it is
          true that one can often get away with using UIViewRoot.createUniqueId()
          as your one-stop-shop for unique ids anywhere in your app, the fact that
          this works is dependent on the particular arrangement of components in a
          view. Let's take a look at UIViewRoot.createUniqueId(), from JSF 1.2
          [1].

          public java.lang.String createUniqueId()

          Generate an identifier for a component. The identifier will be
          prefixed with UNIQUE_ID_PREFIX, and will be unique within this
          UIViewRoot.

          And let's also take a look at UIComponent.setId(), also from JSF 1.2
          [2].

          public abstract void setId(java.lang.String id)

          [...]

          Component identifiers must also obey the following semantic
          restrictions (note that this restriction is NOT enforced by the
          setId() implementation):

          The specified identifier must be unique among all the components
          (including facets) that are descendents of the nearest ancestor
          UIComponent that is a NamingContainer, or within the scope of the
          entire component tree if there is no such ancestor that is a
          NamingContainer.

          Manfred recommended this approach: If you must call setId() with an auto
          generated value, always use the closest ancestor naming container to get
          that value. Given the above constraints, this is the only way to
          guarantee that the auto generated values will not collide.

          I will add the following clarification to UIViewRoot.createUniqueId().

          Previous text:

          The identifier will be prefixed with UNIQUE_ID_PREFIX, and will be
          unique within this UIViewRoot.

          New text:

          The identifier will be prefixed with UNIQUE_ID_PREFIX, and will be
          unique within the non-NamingContainer children of UIViewRoot.

          Ed

          [1] https://maven.java.net/service/local/repositories/releases/archive/com/sun/faces/jsf-api/1.2/jsf-api-1.2-javadoc.jar/!/javax/faces/component/UIViewRoot.html#createUniqueId%28%29

          [2] https://maven.java.net/service/local/repositories/releases/archive/com/sun/faces/jsf-api/1.2/jsf-api-1.2-javadoc.jar/!/javax/faces/component/UIComponent.html#setId%28java.lang.String%29

          Show
          Ed Burns added a comment - Hello Richard and Manfred, Thanks to you both for helping to make JSF more robust. While it is true that one can often get away with using UIViewRoot.createUniqueId() as your one-stop-shop for unique ids anywhere in your app, the fact that this works is dependent on the particular arrangement of components in a view. Let's take a look at UIViewRoot.createUniqueId(), from JSF 1.2 [1] . public java.lang.String createUniqueId() Generate an identifier for a component. The identifier will be prefixed with UNIQUE_ID_PREFIX, and will be unique within this UIViewRoot. And let's also take a look at UIComponent.setId(), also from JSF 1.2 [2] . public abstract void setId(java.lang.String id) [...] Component identifiers must also obey the following semantic restrictions (note that this restriction is NOT enforced by the setId() implementation): The specified identifier must be unique among all the components (including facets) that are descendents of the nearest ancestor UIComponent that is a NamingContainer, or within the scope of the entire component tree if there is no such ancestor that is a NamingContainer. Manfred recommended this approach: If you must call setId() with an auto generated value, always use the closest ancestor naming container to get that value. Given the above constraints, this is the only way to guarantee that the auto generated values will not collide. I will add the following clarification to UIViewRoot.createUniqueId(). Previous text: The identifier will be prefixed with UNIQUE_ID_PREFIX, and will be unique within this UIViewRoot. New text: The identifier will be prefixed with UNIQUE_ID_PREFIX, and will be unique within the non-NamingContainer children of UIViewRoot. Ed [1] https://maven.java.net/service/local/repositories/releases/archive/com/sun/faces/jsf-api/1.2/jsf-api-1.2-javadoc.jar/!/javax/faces/component/UIViewRoot.html#createUniqueId%28%29 [2] https://maven.java.net/service/local/repositories/releases/archive/com/sun/faces/jsf-api/1.2/jsf-api-1.2-javadoc.jar/!/javax/faces/component/UIComponent.html#setId%28java.lang.String%29
          Hide
          kennardconsulting added a comment - - edited

          Okay, I'm very happy with that.

          How about we change the name of this issue to something like 'UIViewRoot.createUniqueId does not always create unique ids' (which, as it turned out, was the cause of all this) and then the resolution is simply a doc clarification?

          So it appears that JAVASERVERFACES-1826 really did resolve the dynamic components issue, after all. Brilliant! Thank you guys so much for sticking with me and getting this working. I apologise for my impatient demeanor at times.

          I'm still doing futher testing, but so far it is looking perfect. I'll open new issues as they arise and as I start porting more apps. But for now you can consider this issue, and JAVASERVERFACES-1826, closed.

          Metawidget finally works properly on Mojarra 2!!!

          Show
          kennardconsulting added a comment - - edited Okay, I'm very happy with that. How about we change the name of this issue to something like 'UIViewRoot.createUniqueId does not always create unique ids' (which, as it turned out, was the cause of all this) and then the resolution is simply a doc clarification? So it appears that JAVASERVERFACES-1826 really did resolve the dynamic components issue, after all. Brilliant! Thank you guys so much for sticking with me and getting this working. I apologise for my impatient demeanor at times. I'm still doing futher testing, but so far it is looking perfect. I'll open new issues as they arise and as I start porting more apps. But for now you can consider this issue, and JAVASERVERFACES-1826 , closed. Metawidget finally works properly on Mojarra 2!!!
          Hide
          Manfred Riem added a comment -

          Closing as "Works as designed" but showing when the issue was really fixed as a consequence of fixing #1826. Note the requested JavaDoc clarification is being tracked as a spec issue.

          Show
          Manfred Riem added a comment - Closing as "Works as designed" but showing when the issue was really fixed as a consequence of fixing #1826. Note the requested JavaDoc clarification is being tracked as a spec issue.
          Hide
          arjan tijms added a comment -

          After further testing, it appears there are some points of contention between how MyFaces and Mojarra handle this. There seem to be cases, when using MyFaces 2 with RichFaces Ajax processing, that UIForm.createUniqueId will just return 'j_id3' (or whatever) without also incrementing the global value returned by UIViewRoot.createUniqueId. This leads to duplicate id conflicts.

          With the new insight, is this issue now also resolved? Or do you use UIViewroot for MyFaces and UIForm for Mojarra now?

          Show
          arjan tijms added a comment - After further testing, it appears there are some points of contention between how MyFaces and Mojarra handle this. There seem to be cases, when using MyFaces 2 with RichFaces Ajax processing, that UIForm.createUniqueId will just return 'j_id3' (or whatever) without also incrementing the global value returned by UIViewRoot.createUniqueId. This leads to duplicate id conflicts. With the new insight, is this issue now also resolved? Or do you use UIViewroot for MyFaces and UIForm for Mojarra now?
          Hide
          Manfred Riem added a comment -

          You should use the nearest UINamingContainer to get a truly unique id within the scope of a UINamingContainer. So you would walk up the tree, get the first UINamingContainer and ask it for a unique id. That should work in both MyFaces and Mojarra.

          Show
          Manfred Riem added a comment - You should use the nearest UINamingContainer to get a truly unique id within the scope of a UINamingContainer. So you would walk up the tree, get the first UINamingContainer and ask it for a unique id. That should work in both MyFaces and Mojarra.
          Hide
          arjan tijms added a comment -

          I hear you, that should work, but I'm still concerned about the statement kennardconsulting made that MyFaces in combination with RichFaces has a problem with that approach. If this is indeed true, then it's of course a MyFaces issue and neither a JSF Spec or Mojarra problem.

          Show
          arjan tijms added a comment - I hear you, that should work, but I'm still concerned about the statement kennardconsulting made that MyFaces in combination with RichFaces has a problem with that approach. If this is indeed true, then it's of course a MyFaces issue and neither a JSF Spec or Mojarra problem.
          Hide
          kennardconsulting added a comment -

          I kind of dodged the issue, since I needed something that would work on JSF1 and JSF2, and didn't want to chase people any further. I'll leave that fight for somebody else

          My 'dodge' is here (FacesUtils.createUniqueId):

          https://github.com/metawidget/metawidget/blob/master/modules/faces/core/src/main/java/org/metawidget/faces/FacesUtils.java

          It seems to work for my purposes, but I'd appreciate anybody's feedback if this is considered 'unsafe'.

          Show
          kennardconsulting added a comment - I kind of dodged the issue, since I needed something that would work on JSF1 and JSF2, and didn't want to chase people any further. I'll leave that fight for somebody else My 'dodge' is here (FacesUtils.createUniqueId): https://github.com/metawidget/metawidget/blob/master/modules/faces/core/src/main/java/org/metawidget/faces/FacesUtils.java It seems to work for my purposes, but I'd appreciate anybody's feedback if this is considered 'unsafe'.

            People

            • Assignee:
              Manfred Riem
              Reporter:
              Manfred Riem
            • Votes:
              6 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 15 minutes
                15m