Component wrapping does not work properly in 2.3.17

[fcjm]

Describe the bug

We recently upgraded our Wildfly to version 26.0.1 which now provides JSF version 2.3.17. We have a case in which we use simple composite components which can wrap more complex components (more specificly the PrimeFaces DataTable). If the wrapping component only consists of simple html-Elements, the wrapped component will not work properly. First I thought this might be a problem with PrimeFaces, so I created a primefaces-test project reproducing the issue (see below). But when starting the project with Mojarra 2.3.17 the problem is reproducable, while with mojarra 2.3.16 it is not. This was causing me to believe that there might be a problem with the current Mojarra 2.3.17 release.

To Reproduce
reproducer.zip

Steps to reproduce the behavior:

1. Start the reproducer via 'mvn clean jetty:run -Pmojarra2317'
2. Go to 'http://localhost:8080/primefaces-test/working.xhtml' (welcome-page)
3. Sort/Filter the DataTable -> works
4. Go to 'http://localhost:8080/primefaces-test/notworking.xhtml'
5. Sort/Filter the DataTable -> does not work

I have also added a profile -Pmojarra2316 which you can use to counter-check that it actually did work with Mojarra 2.3.16.

Expected behavior

Same behaviour as Mojarra 2.3.16 (working)

Desktop (please complete the following information):

• OS: Windows 10
• Browser Chrome
• Version 99.0.4844.51

Additional context

We found a workaround for now which consists of using facelet-tags instead of composite components in this scenario. When doing that, the clientId gets shortened, since the wrapping components Id does no longer show up in it. So I suspect the issue might lie in the detection of components on server-side. Unfortunately though I was not able to find any evidence for this. It would be great if someone with more insight could take a look at it.

[mnriem]

Please remove PrimeFaces from the mix altogether to see if it is Mojarra causing the issue.

[OBreidenbach]

I did some debugging add it seems as this different behavior is because of the new introduced "DescendantMarkIdCache".
Better of it new usage in ComponentSupport#findChildByTagId()

Behavior with Mojarra 2.3.16:
During RESTORE_VIEW: primefaces FilterFeature#decode does extract the filter value from the request and sets it into the datatable object.

During RENDER_RESPONSE: primefaces FilterFeature#filter does take the filter value from the datatable object which is the same as in RESTORE_VIEW.
It is he same because it was found by the implementation of ComponentSupport#findChildByTagId()

Behavior with Mojarra 2.3.17:
During RENDER_RESPONSE the created dataTable objet is not found by ComponentSupport#findChildByTagId() which is no delegating to ComponentSupport#getDescendantMarkIdCache.

The datatable object was added to the Cache as child oft he composite component but was also removed afterwards.

If the datatable inside the composite component is inside another faces component, the datatable component is found by ComponentTagHandlerDelegateImpl#findReparentedComponent method.

Maybe @BalusC could have a short lock, if the cache is working properly?

[OBreidenbach]

I created a reproducer without primefaces, so it is still the testcase provided from primefaces but I removed all decencies to primefaces.
reproducr_without_primefaces.zip

This reproducer uses just a <h:selectOneListbox>, but it could be any component.
As I said in my former comment, the component object instance is not found in the cache and so a new one is created. As some primefaces components keep state in the object instance during a request, the state got lost.

My example contains 2 views.

• /test/sendEntireForm.xhtml
• /test/sendOnChange.xhtml

In the logging of both you will see, that for the component which is wrapped in the 'htmlWrapper' during RENDER_RESPONSE a new object instance is created. This does not happen, if no composite component is used or there is a other component in the composite component.

This only happens with Mojarra 2.3.17, it does not happen with Mojarra 2.3.16. There the object instance stays the same during the request.

Logging with Mojarra 2.3.16:

PhaseID	Direct	ComponentWrapper	HtmlWrapper
RESTORE_VIEW	Component@765b1a51	Component@6e0c7607	Component@3f202975
APPLY_REQUEST_VALUES	Component@765b1a51	Component@6e0c7607	Component@3f202975
PROCESS_VALIDATIONS	Component@765b1a51	Component@6e0c7607	Component@3f202975
UPDATE_MODEL_VALUES	Component@765b1a51	Component@6e0c7607	Component@3f202975
INVOKE_APPLICATION	Component@765b1a51	Component@6e0c7607	Component@3f202975
RENDER_RESPONSE	Component@765b1a51	Component@6e0c7607	Component@3f202975

Logging with Mojarra 2.3.17:

PhaseID	Direct	ComponentWrapper	HtmlWrapper
RESTORE_VIEW	Component@34d1bf88	Component@efc1a50	Component@22e2c614
APPLY_REQUEST_VALUES	Component@34d1bf88	Component@efc1a50	Component@22e2c614
PROCESS_VALIDATIONS	Component@34d1bf88	Component@efc1a50	Component@22e2c614
UPDATE_MODEL_VALUES	Component@34d1bf88	Component@efc1a50	Component@22e2c614
INVOKE_APPLICATION	Component@34d1bf88	Component@efc1a50	Component@22e2c614
RENDER_RESPONSE	Component@34d1bf88	Component@efc1a50	Component@4d63129

So with Mojarra 2.3.17 a new component instance was create during RENDER_RESPONSE if the component is inside the htmlWrapper composite component. This does not happen with Mojarra 2.3.16.

[mnriem]

Probably a regression because of #4934

[BalusC]

Excellent analysis/reproducer, very helpful. And yes Manfred this indeed looks like to be a regression of this original work: #4808

[BalusC]

Okay, nailed down. This does at first glance not seem to be a regression but just an existing bug being uncovered which was previously seemingly worked around with the original inefficient implementation of ComponentSupport#findChildByTagId().

ComponentTagHandlerDelegateImpl#apply() looks like this: https://github.com/eclipse-ee4j/mojarra/blob/2.3.17-RELEASE/impl/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentTagHandlerDelegateImpl.java#L120

        // grab our component
        UIComponent c = findChild(ctx, parent, id);
        if (null == c &&
            context.isPostback() &&
            UIComponent.isCompositeComponent(parent) &&
            parent.getAttributes().get(id) != null) {
            c = findReparentedComponent(ctx, parent, id);
        }

Technically speaking, the <h:selectOneListbox> in the reproducer is reparented (i.e. handled via InsertChildrenHandler). So, during render response of a postback, you'd expect findChild() which delegates to ComponentSupport#findChildByTagId() to return null and then the logic will eventually find it via findReparentedComponent(). But this did not happen in 2.3.16 and before. It was found right away via findChild().

Since the performance optimizations of ComponentSupport#findChildByTagId() introduced in 2.3.17, the findChild() returns null, which is completely expected, because the <h:selectOneListbox> is reparented, but the findReparentedComponent() unexpectedly returned null. And then the logic will create a new component, which is incorrect and explains the observable problems.

As to reparenting, during view build time of postback, the composite component basically creates during CompositeComponentTagHandler#applyCompositeComponent() an UIPanel representing the <cc:implementation>: https://github.com/eclipse-ee4j/mojarra/blob/2.3.17-RELEASE/impl/src/main/java/com/sun/faces/facelets/tag/jsf/CompositeComponentTagHandler.java#L321

        if (ComponentHandler.isNew(c)) {
            facetComponent = (UIPanel)
            facesContext.getApplication().createComponent("javax.faces.Panel");
            facetComponent.setRendererType("javax.faces.Group");
            c.getFacets().put(UIComponent.COMPOSITE_FACET_NAME, facetComponent);
        }                                                                                 

If its direct child is a <cc:insertChildren> then these children are reparented against this UIPanel. The reparenting is tracked in InsertChildrenHandler#processEvent(): https://github.com/eclipse-ee4j/mojarra/blob/2.3.17-RELEASE/impl/src/main/java/com/sun/faces/facelets/tag/composite/InsertChildrenHandler.java#L154

            //store the new parent's info per child in the old parent's attr map
            //<child id, new parent>
            for (UIComponent c : compositeChildren) {
                String key =  (String)c.getAttributes().get(ComponentSupport.MARK_CREATED);
                String value = component.getId(); // <-- this unexpectedly returned null
                if (key != null && value != null) {
                    compositeParent.getAttributes().put(key, value);
                }
            }

In case of the UIPanel, the component variable is exactly the same as facetComponent created earlier. However, its getId() returned null. This has as consequence that ComponentTagHandlerDelegateImpl#apply() won't invoke findReparentedComponent() because parent.getAttributes().get(id) returned null.

        // grab our component
        UIComponent c = findChild(ctx, parent, id);
        if (null == c &&
            context.isPostback() &&
            UIComponent.isCompositeComponent(parent) &&
            parent.getAttributes().get(id) != null) { // <-- condition not satisfied here
            c = findReparentedComponent(ctx, parent, id);
        }

The fix was explicitly set ID of UIPanel in CompositeComponentTagHandler#applyCompositeComponent() as can be seen in commit 27f7f0b

Will add IT later after I've verified that all existing 2.3 tests pass.

[OBreidenbach]

@BalusC, thank you for the fixing this so fast.