search

eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
158 stars 107 forks source link

hidden field autocomplete="off" is not anymore valid #5434

Closed BalusC closed 2 months ago

BalusC commented 2 months ago

Originally reported in Faces project: https://github.com/jakartaee/faces/issues/1912

As per latest HTML spec the autocomplete="off" is not anymore valid on hidden fields (e.g. view state, client window state).

It was originally introduced to work around Firefox specific behavior which was not observable on other browsers: https://github.com/eclipse-ee4j/mojarra/issues/1133

Reproducer:

  1. Open Faces page with Ajax form (and inspect jakarta.faces.ViewState hidden input field and remember its value).
  2. Perform Ajax submit.
  3. Hard-Refresh page and inspect jakarta.faces.ViewState hidden input field. It should have a new value, not the one from the initially opened page.

This happened only in Firefox as per https://bugzilla.mozilla.org/show_bug.cgi?id=520561 and work around was to add autocomplete="off" to the hidden field.

Fast forward ~15 years, the specific attribute value of off is considered invalid as per Nu HTML validator https://validator.w3.org/nu/ as per recent changes in HTML spec https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#autofill

We can turn off this work around via following context param:

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

The original problem is also not anymore reproducible in current Firefox versions (tested 124.0.2) so I think it's OK to make this config the default as per Mojarra 4.1. I.e. don't anymore render autocomplete="off" unless explicitly instructed as follows:

<context-param>
    <param-name>com.sun.faces.autoCompleteOffOnViewState</param-name>
    <param-value>true</param-value>
</context-param>
pizzi80 commented 2 months ago

to simplify Faces configuration this could also be deprecated for removal... ?

BalusC commented 2 months ago

Yeah, it could indeed be deprecated for removal. But it's not part of API ;)

pizzi80 commented 2 months ago

so we could delete the property and the associated logic ... ;P

BalusC commented 2 months ago

I'd rather not. Imagine that Firefox or any other browser somehow decides to resurrect this misbehavior. I think it's in long term better to change the autofill type. one-time-code sounds ideal but it's broken in older Firefox versions. Probably new-password but I have a very awkward feeling with this.

melloware commented 2 months ago

Yeah its easy enough to leave in the code. Its off by default now so its not harming anything to leave in.