Revist hidden field autocomplete="off"

[volosied]

@BalusC You said in Step 3: It should have a new value, not the one from the initially opened page.

I found that the new value is used when the page is refreshed in FireFox. However, in Safari / Chrome, the original value is displayed:

Pure HTML/JS code to replicate

      <form><input type=hidden name="test" value="original"></form>
      <button id=show>show hidden value</button><button
         id=update>Update value</button>
      <script>document.getElementById("show").addEventListener("click", () => { alert(document.querySelector("input").value) }); document.getElementById("update").addEventListener("click", () => { document.querySelector("input").value = "updated" })</script>

I was also able to reproduce the behavior above via JSF (client side state saving and an input within a c:forEach). The scenario I tested likely isn't very common, but it still proves that hidden field change is still possible.

I'm wondering which browsers are correct? Is it Firefox or the others. This is a behavior difference, and I could still see someone affected by this difference in functionality between browsers.

I'm leaning to revert this change if it can somehow break an application/users, as the HTML validation error is less severe.

Originally posted by @volosied in #5434

[volosied]

Created a new issue, so that my comment wouldn't get lost.

[volosied]

@melloware Any objection if I change this back in MyFaces? As I mentioned, it seems like this bug is still possible (though I don't know which browser behavior is 'correct'). That said, I prefer consistency across browsers when autocomplete=off is added to Firefox.

If someone really doesn't need it they can use a parameter to change it, but at least we're reducing the possibility of an app/site breaking for a user.

Let me know your thoughts.

[BalusC]

How about setting an explicit autofill type such as one-time-code?

[volosied]

That seems to work in my testing:

<input type="hidden" name="jakarta.faces.ViewState" id="j_id__v_0:jakarta.faces.ViewState:1" value="pLXoDt3ZfnKp3jWq...ENXBOuo=" autocomplete="one-time-code">

When refreshing the page in FireFox, the original viewstate is displayed, not the updated one. This is consistent across the three browsers I tested.

Seems like a reasonable fix? Reading the top answer here, it seems like a known workaround. Though, would there any unintended consequences?

[BalusC]

Okay. We can try that. And then

<context-param>
    <param-name>com.sun.faces.autoCompleteOffOnViewState</param-name>
    <param-value>true</param-value>
</context-param>

will use off instead of one-time-code.

[volosied]

I thought we would go with one-time-code? Shouldn't it be vice versa - one-time-code instead of off?

We'll still support off/false and on/true? Correct? But, one-time-code would be the default?

[BalusC]

one-time-code doesn't work in older browsers (about 2021) so there should still be a way to set it to off.

Less context params is better and default rendering must be HTML-valid.

Or do you wish a way to completely remove the autocomplete attribute from view state hidden field? If so how is that useful? Note that the context param was originally introduced because the original solution was HTML-invalid.

[volosied]

I'd rather keep autocomplete=off, even if it's HTML-invalid. The risk of breaking a user is higher than having invalid HTML. If validation is a concern, they can just change the context parameter (assuming we document the risk).

[BalusC]

If you want to keep autocomplete=off in 4.1 just add

<context-param>
    <param-name>com.sun.faces.autoCompleteOffOnViewState</param-name>
    <param-value>true</param-value>
</context-param>

It hasn't been removed, the default has just been inversed.

[volosied]

I understand, but, the new default could potentially cause problems to users. Mojarra could leave the new default alone, but, perhaps, the MyFaces community can change it back to false.

As I mentioned, I think the HTML validation error is insignificant compared to a application misbehaving if a auto-completed viewstate string is used instead.

My priority is to have consistent behavior across browsers, and autcomplete=off fixes the issue in FireFox. Though, this may be slightly different from the original issue?

The issue can be exhibited via this standalone code:

      <form><input type=hidden name="test" value="original"></form>
      <button id=show>show hidden value</button><button
         id=update>Update value</button>
      <script>document.getElementById("show").addEventListener("click", () => { alert(document.querySelector("input").value) }); document.getElementById("update").addEventListener("click", () => { document.querySelector("input").value = "updated" })</script>

If you test Chrome and Firefox, you'll see a different value after you refresh the page. Firefox displays the 'updated' value after the refresh while Chome/Safari show the original value.

Perhaps I'm overthinking this.

[BalusC]

I just prepped a PR to use autocomplete=one-time-code by default.
The context param can still be used to set that to autocomplete=off in case you wish to support legacy browsers.
How about that?

(sorry, this overlapped with your previous comment)

[melloware]

@volosied i like this one-time-code option that @BalusC came up with. We should do this for Myfaces.

[volosied]

@melloware

Perhaps, we can have a balance.
For 4.0 and earlier, we’ll use autocomplete=off as default (just as before) and add one-time-code as a new option. This would allow users to have valid HTML if they wanted to but with the caveat that one-time-code is only supported in new browsers. Also potentially avoids breaking any users who upgrade to newer releases.

For 4.1, we’ll make one-time-code as the default but still allow users to select on / off as values. (edited)