FACELETS_REFRESH_PERIOD > 0 triggers incorrect rendering

[rmartinc]

Describe the bug

If the option javax.faces.FACELETS_REFRESH_PERIOD is configured in the web.xml to something greater than 0 it makes that some attributes are not rendered OK for the components. In the attached application the HtmlForm loses the style attribute.

To Reproduce

Compile and deploy the attached app:

unzip test-jsf.zip
cd test-jsf
mvn clean install

Go to the test page, http://127.0.0.1:8080/test-jsf/test.xhtml and click the button two times. The style for the text is different. Check the form in the html page and it has no style attribute.

Expected behavior

The form should maintain the style attribute.

Desktop (please complete the following information):

• OS: All
• Browser All
• Version All

Additional context

Looking the code the problem is that the DefaultFacelet.java adds a timestamp when using the refresh time to remove old elements (here). It happens that the form uses an optimized rendering that renders the attrs in attributesThatAreSet. Those seem to be removed when adding the timestamp.

@arjantijms @BalusC Don't we need to initiate the list of the delta like this?

diff --git a/impl/src/main/java/jakarta/faces/component/ComponentStateHelper.java b/impl/src/main/java/jakarta/faces/component/ComponentStateHelper.java
index 96d4cd88..e7de3068 100644
--- a/impl/src/main/java/jakarta/faces/component/ComponentStateHelper.java
+++ b/impl/src/main/java/jakarta/faces/component/ComponentStateHelper.java
@@ -210,14 +210,14 @@ class ComponentStateHelper implements StateHelper, TransientStateHelper {
     }
 
     private void initList(Serializable key) {
-        if (component.initialStateMarked()) {
-            deltaMap.computeIfAbsent(key, e -> new ArrayList<>(4));
-        }
-
         if (get(key) == null) {
             List<Object> items = new ArrayList<>(4);
             defaultMap.put(key, items);
         }
+
+        if (component.initialStateMarked()) {
+            deltaMap.computeIfAbsent(key, e -> new ArrayList<>((List<Object>) get(key)));
+        }
     }

With that change I'm initializing the delta with the contents of the default, and then adding the new value. If not the delta only contains the added attribute but loses the previous ones. And the restore only resets the timestamp losing the original style.

I can send a PR if you want or you can add the change. I tested with version 2.3 and I see no regression.

Thanks in advance!

test-jsf.zip

[arjantijms]

Looks reasonable at a glance. Since we're just working in the 4.0 TCK now, test-jsf.zip could be made into a TCK test as well. @BalusC what do you think?

[rmartinc]

If you want I can change the little app into a jakarta one and trying to create a test-app (or add the xhtml files to a existing one). But I cannot even compile the master branch right now cos the artifacts are not in maven (jakarta.servlet:jakarta.servlet-api:jar:6.0.0 for example). I suppose some other dev repo should be used.

[pizzi80]

can't reproduce on Faces 3.0.2 with the following conf: 🤔

<context-param>
    <param-name>jakarta.faces.FACELETS_BUFFER_SIZE</param-name>
    <param-value>131072</param-value> 
</context-param>
<context-param>
    <param-name>jakarta.faces.STATE_SAVING_METHOD</param-name>
    <param-value>server</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.PROJECT_STAGE</param-name>
    <param-value>Production</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.FACELETS_REFRESH_PERIOD</param-name>
    <param-value>1</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.FACELETS_SKIP_COMMENTS</param-name>
    <param-value>true</param-value>
</context-param>

[rmartinc]

Hi @pizzi80 !

I can reproduce the error in glassfish 6.2.3 (mojarra 3.0.1) and copying 3.0.2 into ${GLASSFISH_HOME}/glassfish/modules/jakarta.faces.jar. Attached my little project modified to be a jakarta one. If you click in the button several times (two) the style is missing in the form.

With my little change the style is not missed. Also tested.

test-jsf-jakarta.zip

[pizzi80]

very curious!
I'm testing on Tomcat 10.0.16 with Java 17

with the following java and xhtml snippets
the form colors never change even after a lot of rapid click 🤯

I've tested also with ajax

@Named
@ViewScoped
public class TestAction implements Serializable {

    private static final long serialVersionUID = 2806325319047342503L;

    private static final Logger log = Logger.getLogger(TestAction.class.getName());

    private String value;
    public String getValue() {return value;}
    public void setValue(String value) {this.value = value;}

    @PostConstruct
    public void init() {
        value = "hello";
        log.info("init with value "+value);
    }

    public void submit() {
        log.info("submit()");
    }

}

<h:form id="testHtmlAttributes" style="border: 3px solid red;" styleClass="foo bar baz">
    <div class="container">
        <div class="form-group">
            <h:commandButton id="run" value="Test with request" actionListener="#{testAction.submit}" />
            <h:commandButton id="runAjax" value="Test with ajax" action="#{testAction.submit}" >
                <f:ajax execute="@form" render="@form" />
            </h:commandButton>
            <h:panelGrid columns="4" cellpadding="5" class="ui-grid" style="background-color: yellow;">
                <h:outputText value="#{testAction.value}" style="color: blue;" /><br/>
            </h:panelGrid>
        </div>
    </div>
</h:form>

[rmartinc]

My test also fails in tomcat 10 and jdk 17. Note that the failure also depends in how the xhtml files are included and templated (when the timestamp is added). If you are testing with another app it can work OK. But my test fails in every jsf 3.0.2 no matter the jdk or the container. Please test with my reproducer.

[pizzi80]

You're right!
At first I didn't understand that the problem was the template composition

reproduced also with ajax, the attributes are missing inside the response

[mnriem]

@rmartinc Sending a PR with it all would be a good way to go!

[rmartinc]

I'll send a PR the next week. I could compile master branch adding repos from https://jakarta.oss.sonatype.org. But I see that now in master all the test folder is removed. So, if nobody says something different, I'll just send the little modification commented before. Thanks!

[rmartinc]

@arjantijms @mnriem I submitted PR #5062. Thanks!

[arjantijms]

But I see that now in master all the test folder is removed.

That's correct, the tests have been moved to https://github.com/jakartaee/faces/tree/master/tck

They were always intended and setup to be implementation neutral, so we used them as base for the new TCK.

[BalusC]

Should also be fixed in 2.3 and 3.0.

[melloware]

I have added backports for 2.3 and 3.0 PRs