Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore the problematics HTML validation checks with hidden inputs #10020

Conversation

ahukkanen
Copy link
Contributor

@ahukkanen ahukkanen commented Nov 2, 2022

🎩 What? Why?

Rails adds autocomplete="off" automatically to hidden inputs which is not valid HTML. This causes the following HTML validation issue after a recent update to the HTML validator:

  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”.

But we need to preserve this functionality for the time being because of a Firefox bug that is fixed using this workaround.

For the time being we will ignore this check until we will find a proper solution for this (see the related issue). The approach for ignoring the validation issue originates from #8597 (which we removed as unnecessary at #9878).

📌 Related Issues

Testing

See that CI is green or run these manually:

$ 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

Rails adds `autocomplete="off"` automatically to hidden inputs
which is not valid HTML.

But we need to preserve this functionality for the time being
because of a Firefox bug that is fixed using this workaround.
@ahukkanen ahukkanen added module: dev 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 2, 2022
@ahukkanen
Copy link
Contributor Author

ahukkanen commented Nov 2, 2022

Note: the aria-* attribute issue is caused by Foundation Abide and I'm fixing that for Foundation here:
foundation/foundation-sites#12496

@ahukkanen
Copy link
Contributor Author

ahukkanen commented Nov 2, 2022

So the actual issue why this is working for me locally and why it is broken in the CI is that locally I'm using the online HTML validator and CI is using the docker packaged version. The online validator is not currently up-to-date with the docker packaged version.

This is the assertion that is supposed to be failing:
validator/validator@84bde80

But it had a bug in it which was fixed today:
validator/validator@b896708

So that last change is already published to the validator version we are using at CI. But the online validator is not yet updated which causes it to be still broken in the online version.

Example HTML to test out this issue with the online validator:

<!DOCTYPE html>
<html lang="">
<head>
  <title>Test</title>
</head>
<body>
  <form>
    <input type="hidden" name="test" value="abc" aria-describedby="test-abide-error">
    <span id="test-abide-error">Test</span>
  </form>
</body>
</html>

It will actually evaluate as valid due to the second change which is not yet published to the online validator (that I'm using locally).

@ahukkanen ahukkanen removed the type: fix PRs that implement a fix for a bug label Nov 2, 2022
@andreslucena andreslucena changed the title Ignore the problematic HTML validation check with Rails Ignore the problematics HTML validation checks with hidden inputs Nov 2, 2022
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment to be added and it'd be good for me. I've also changed the PR title to better reflect the latest changes, feel free to improve on it!

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
@ahukkanen
Copy link
Contributor Author

Done @andreslucena.

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
@ahukkanen
Copy link
Contributor Author

ahukkanen commented Nov 2, 2022

Note: We only have to backport this to 0.27.

Backporting to 0.26 is not necessary because:

  1. The autocomplete="off" was added in Rails 6.1 (which we use for 0.27)
  2. The aria-* on hidden inputs only happen when running the specs under redesigned layout, so the second issue only applies to develop

Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽

@andreslucena andreslucena merged commit 449a6a2 into decidim:develop Nov 3, 2022
@ahukkanen ahukkanen deleted the fix/ignore-html-validation-hidden-input-autocomplete branch November 3, 2022 08:43
entantoencuanto added a commit that referenced this pull request Nov 3, 2022
* develop:
  Ignore the problematics HTML validation checks with hidden inputs (#10020)
entantoencuanto added a commit that referenced this pull request Nov 3, 2022
* develop:
  Ignore the problematics HTML validation checks with hidden inputs (#10020)
  Fix proposal etiquette and length validator with base64 images (#9639)
entantoencuanto added a commit that referenced this pull request Nov 3, 2022
* develop:
  Ignore the problematics HTML validation checks with hidden inputs (#10020)
  Fix proposal etiquette and length validator with base64 images (#9639)
entantoencuanto added a commit that referenced this pull request Nov 3, 2022
* develop:
  Ignore the problematics HTML validation checks with hidden inputs (#10020)
  Fix proposal etiquette and length validator with base64 images (#9639)
entantoencuanto added a commit that referenced this pull request Nov 3, 2022
* develop:
  Ignore the problematics HTML validation checks with hidden inputs (#10020)
Quentinchampenois pushed a commit to Quentinchampenois/decidim that referenced this pull request Nov 23, 2022
…cidim#10020)

* Ignore the problematic HTML validation check with Rails

Rails adds `autocomplete="off"` automatically to hidden inputs
which is not valid HTML.

But we need to preserve this functionality for the time being
because of a Firefox bug that is fixed using this workaround.

* Prevent aria-describedby attribute being added to hidden inputs

* Revert "Prevent aria-describedby attribute being added to hidden inputs"

This reverts commit 5a90162.

* Ignore the aria-* attributes rule with hidden inputs

* Add reference to the foundation issue

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>

* Remove unnecessary whitespace

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: dev 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.

None yet

2 participants