Skip to content

Remove autocomplete attribute from hidden inputs#10014

Closed
ahukkanen wants to merge 1 commit intodecidim:developfrom
mainio:fix/hidden-field-a11y
Closed

Remove autocomplete attribute from hidden inputs#10014
ahukkanen wants to merge 1 commit intodecidim:developfrom
mainio:fix/hidden-field-a11y

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Nov 1, 2022

🎩 What? Why?

Since the recent HTML validator update, some validation specs started failing because Rails helpers add the autocomplete="off" attribute automatically to hidden input elements. This is no longer allowed, so this PR fixes the issue by removing that attribute from hidden inputs by customizing the Rails helpers.

The errors you would see in the system specs look as follows:

  1) Participatory Processes when there are some published processes when requesting the processes path with highlighted processes behaves like accessible page passes HTML validation
     Failure/Error: DEFAULT_FAILURE_NOTIFIER = lambda { |failure, _opts| raise failure }
     
       ######
       line 375: An “input” element with a “type” attribute whose value is “hidden” must not have an “autocomplete” attribute whose value is “on” or “off”.

📌 Related Issues

Testing

$ cd decidim-core
$ bundle exec rpsec spec/system/messaging/conversations_spec.rb
$ cd ../decidim-proposals
$ bundle exec rspec spec/system/proposals_spec.rb
$ bundle exec rspec spec/system/amendable/amendment_diff_spec.rb
$ cd ../decidim-participatory_processes
$ bundle exec rspec spec/system/participatory_processes_spec.rb

This causes the a11y checks to fail because autocomplete attribute
is not allowed on hidden inputs.
@ahukkanen ahukkanen added module: core type: fix PRs that implement a fix for a bug type: internal PRs that aren't necessary to add to the CHANGELOG for implementers labels Nov 1, 2022
@ahukkanen
Copy link
Copy Markdown
Contributor Author

Actually this may be problematic with Firefox, as I was investigating where this originates from:
rails/rails#42610

As reported/diagnosed in May of WTFs: https://discuss.rubyonrails.org/t/form-with-first-field-value-is-overriden-with-a-token-like-string/74861/11 there is a 12-year-old Firefox bug that (sporadically, but pretty frequently) overwrites the first hidden_field in a form UNLESS it has autocomplete=OFF

@andreslucena
Copy link
Copy Markdown
Member

Actually this may be problematic with Firefox, as I was investigating where this originates from:

I was just going to add that link 😄

Not sure how to proceed: should we ignore this a11y rule (and have invalid HTML) or have this problem with Firefox browsers (as the longstanding bug its still open)? Or maybe here's a third way that I'm no seeing?

@ahukkanen
Copy link
Copy Markdown
Contributor Author

Not sure how to proceed: should we ignore this a11y rule (and have invalid HTML) or have this problem with Firefox browsers (as the longstanding bug its still open)?

I think the first step is here: rails/rails#46405

I will see how we can exclude this check. Sorry, I wrote it incorrectly in the original PR description that this would be caused by the a11y tool. This is actually caused by the HTML validator (as you properly noticed above).

I think we probably need to re-introduce the HTML validation exclusion approach that we removed at #9878 😞

I'll make another PR for that and I'll mark this one as draft.

@ahukkanen
Copy link
Copy Markdown
Contributor Author

I opened another PR to ignore this validation error for the time being at #10020.

We'll see how the Rails issue progresses. It's kind of problematic because we probably need this workaround in Rails until Mozilla fixes the problem in Firefox. Which may take some time as the FF issue was opened 13 years ago...

@Crashillo Crashillo mentioned this pull request Nov 2, 2022
@ahukkanen
Copy link
Copy Markdown
Contributor Author

As decided by maintainers, I will close this PR for now as there is not much we can do about it.

The unfortunate thing about producing invalid HTML that it has to be pinpointed in the accessibility report which pages are not totally accessible (in this case would fail due to invalid HTML). And this would basically include any page that has forms, which is basically all pages in Decidim when signed out because every page has the sign in form in the hidden popup. And after signed in, it would be all pages that have forms, such as proposals listing, submitting a proposal, private messaging, any page with comments, etc.

But there is not much we can do about this until the Firefox bug is fixed, which does not seem likely at the moment. One thing I suggested at the related rails issue was to do this conditionally only for Firefox preserving valid HTML for all other browsers but they did not see it as a good solution because browser detection on the server side is not always reliable and we would potentially cause issues in case the forms are cached. Although to me it sounds weird that the form would be cached with the authenticity tokens which would also break the forms. Maybe there is some magic at Rails side that I'm not understanding regarding the caching part.

@ahukkanen ahukkanen closed this Jun 8, 2023
@ahukkanen ahukkanen deleted the fix/hidden-field-a11y branch June 8, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: core type: fix PRs that implement a fix for a bug type: internal PRs that aren't necessary to add to the CHANGELOG for implementers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants