Fix invalid HTML in comment votes #5426

javierm · 2024-03-04T23:40:40Z

References

We introduced this bug in commit ba0d21b from pull request Make buttons to vote more accessible #4776

Objectives

Avoid any possible issues with the buttons to vote comments due to invalid HTML

Notes

I've considered adding a validation using the w3c_rspec_validators gem (which is based on w3c_validators). However, using it would mean tests would need to connect to the W3C Markup Validation Service, which could cause our CI to randomly fail. For example, I'm getting "Too Many Requests" errors while testing whether all our components generate valid HTML.

In any case, here's the patch introducing a validation with w3c_rspec_validators, just in case we change our minds in the future.

diff --git a/Gemfile b/Gemfile
index b5d7672c23..48cdf7afe7 100644
--- a/Gemfile
+++ b/Gemfile
@@ -90,6 +90,7 @@ group :test do
   gem "selenium-webdriver", "~> 4.16.0"
   gem "simplecov", "~> 0.22.0", require: false
   gem "simplecov-lcov", "~> 0.8.0", require: false
+  gem "w3c_rspec_validators", "~> 0.3.0"
 end
 
 group :development do
diff --git a/Gemfile.lock b/Gemfile.lock
index 8786ce8630..d4e37f2d5e 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -509,6 +509,10 @@ GEM
       parallel (< 2.0)
       public_suffix (>= 2.0.5, < 5.0)
       rack (>= 1.3.6, < 3.0)
+    rspec (3.11.0)
+      rspec-core (~> 3.11.0)
+      rspec-expectations (~> 3.11.0)
+      rspec-mocks (~> 3.11.0)
     rspec-core (3.11.0)
       rspec-support (~> 3.11.0)
     rspec-expectations (3.11.0)
@@ -660,6 +664,14 @@ GEM
       activesupport (>= 5.2.0, < 8.0)
       concurrent-ruby (~> 1.0)
       method_source (~> 1.0)
+    w3c_rspec_validators (0.3.0)
+      rails
+      rspec
+      w3c_validators
+    w3c_validators (1.3.7)
+      json (>= 1.8)
+      nokogiri (~> 1.6)
+      rexml (~> 3.2)
     warden (1.2.9)
       rack (>= 2.0.9)
     wasabi (3.7.0)
@@ -788,6 +800,7 @@ DEPENDENCIES
   uglifier (~> 4.2.0)
   uuidtools (~> 2.2.0)
   view_component (~> 3.6.0)
+  w3c_rspec_validators (~> 0.3.0)
   web-console (~> 4.2.1)
   whenever (~> 1.0.0)
   wicked_pdf (~> 2.7.0)
diff --git a/spec/components/comments/votes_component_spec.rb b/spec/components/comments/votes_component_spec.rb
index 5c373b0519..b34d2e5d73 100644
--- a/spec/components/comments/votes_component_spec.rb
+++ b/spec/components/comments/votes_component_spec.rb
@@ -8,9 +8,7 @@ describe Comments::VotesComponent do
   it "generates valid HTML" do
     render_inline component
 
-    expect(page).not_to have_css "span form"
-    expect(page).to have_css "div.in-favor > form"
-    expect(page).to have_css "div.against > form"
+    expect(page).to have_valid_html
   end
 
   describe "aria-pressed and method attributes" do
diff --git a/spec/support/matchers/have_valid_html.rb b/spec/support/matchers/have_valid_html.rb
new file mode 100644
index 0000000000..295effa688
--- /dev/null
+++ b/spec/support/matchers/have_valid_html.rb
@@ -0,0 +1,31 @@
+RSpec::Matchers.define :have_valid_html do
+  define_method :html do
+    if page.respond_to?(:html)
+      if page.html.strip.starts_with?("<!DOCTYPE")
+        page.html
+      else
+        "<!DOCTYPE html>\n#{page.html}"
+      end
+    elsif page.respond_to?(:native)
+      page.native.inner_html
+                 .sub("<html>", "<!DOCTYPE html><html lang=\"#{I18n.locale}\"><head><title>Title</title>")
+                 .sub("<body>", "</head><body>")
+    else
+      raise "The `page` object should either respond to the `html` method (like Capybara::Session) " \
+            "or to the `native` method (like `Capybara::Node::Simple`, used in components)"
+    end
+  end
+
+  validator = W3cRspecValidators::Validator.new
+
+  match do
+    validator.validate_html(html)
+    validator.response.errors.length == 0
+  end
+
+  failure_message do
+    validator.response.errors.map do |err|
+      W3cRspecValidators::ErrorParser.parse_html_error(err, html)
+    end.join("\n")
+  end
+end

Another option would be to use the html_validation gem, which doesn't connect to an external service, but requires a local installation of Tidy, meaning we'd add an extra dependency in order to run the tests. Also note that, in this case, we get both false positives and false negatives.

A possible implementation based on this gem would be:

diff --git a/.dockerignore b/.dockerignore
index c9b8c5bec1..2f9e224167 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -37,3 +37,4 @@ node_modules/
 # Test results
 spec/examples.txt
 coverage/
+spec/.validation/
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index baf23d7a4c..52a8d8eb3c 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -52,6 +52,8 @@ jobs:
         run: bundle exec rake db:setup
       - name: Compile assets
         run: bundle exec rake assets:precompile > /dev/null 2>&1
+      - name: Install tidy
+        run: sudo apt install tidy
       - name: Run test suite
         env:
           KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC: ${{ secrets.KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC }}
diff --git a/.gitignore b/.gitignore
index ee42703e89..08d80f8c59 100644
--- a/.gitignore
+++ b/.gitignore
@@ -39,3 +39,4 @@ tmp/
 # Test results
 /spec/examples.txt
 /coverage/
+/spec/.validation/
diff --git a/Gemfile b/Gemfile
index b5d7672c23..9fbcc03caa 100644
--- a/Gemfile
+++ b/Gemfile
@@ -85,6 +85,7 @@ group :test do
   gem "capybara", "~> 3.40.0"
   gem "capybara-webmock", "~> 0.7.0"
   gem "email_spec", "~> 2.2.2"
+  gem "html_validation", "~> 1.1.6"
   gem "pdf-reader", "~> 2.12.0"
   gem "rspec-rails", "~> 5.1.2"
   gem "selenium-webdriver", "~> 4.16.0"
diff --git a/Gemfile.lock b/Gemfile.lock
index 8786ce8630..8a87a2c0f2 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -264,6 +264,7 @@ GEM
     hashery (2.1.2)
     hashie (5.0.0)
     highline (2.0.3)
+    html_validation (1.1.6)
     htmlentities (4.3.4)
     httparty (0.21.0)
       mini_mime (>= 1.0.0)
@@ -731,6 +732,7 @@ DEPENDENCIES
   graphiql-rails (~> 1.8.0)
   graphql (~> 1.12.14)
   groupdate (~> 6.4.0)
+  html_validation (~> 1.1.6)
   i18n-tasks (~> 0.9.37)
   image_processing (~> 1.12.2)
   initialjs-rails (~> 0.2.0.9)
diff --git a/README.md b/README.md
index d13143329f..69ef904270 100644
--- a/README.md
+++ b/README.md
@@ -38,7 +38,7 @@ You can access the main website of the project at [http:https://consuldemocracy.org](h
 
 **NOTE**: For more detailed instructions check the [docs](https://docs.consuldemocracy.org)
 
-Prerequisites: install git, Ruby 3.1.4, CMake, pkg-config, shared-mime-info, Node.js 18.18.2 and PostgreSQL (>=9.5).
+Prerequisites: install git, Ruby 3.1.4, CMake, pkg-config, shared-mime-info, the `tidy` HTML syntax checker, Node.js 18.18.2 and PostgreSQL (>=9.5).
 
 ```bash
 git clone https://github.com/consuldemocracy/consuldemocracy.git
diff --git a/README_ES.md b/README_ES.md
index fff0bebfbd..2dfc6f3650 100644
--- a/README_ES.md
+++ b/README_ES.md
@@ -36,7 +36,7 @@ Puedes acceder a la página principal del proyecto en [http:https://consuldemocracy.or
 
 **NOTA**: para unas instrucciones más detalladas consulta la [documentación](https://docs.consuldemocracy.org)
 
-Prerequisitos: tener instalado git, Ruby 3.1.4, CMake, pkg-config, shared-mime-info, Node.js 18.18.2 y PostgreSQL (9.5 o superior).
+Prerequisitos: tener instalado git, Ruby 3.1.4, CMake, pkg-config, shared-mime-info, el comprobador de sintaxis HTML `tidy`, Node.js 18.18.2 y PostgreSQL (9.5 o superior).
 
 ```bash
 git clone https://github.com/consuldemocracy/consuldemocracy.git
diff --git a/spec/components/comments/votes_component_spec.rb b/spec/components/comments/votes_component_spec.rb
index 5c373b0519..b34d2e5d73 100644
--- a/spec/components/comments/votes_component_spec.rb
+++ b/spec/components/comments/votes_component_spec.rb
@@ -8,9 +8,7 @@ describe Comments::VotesComponent do
   it "generates valid HTML" do
     render_inline component
 
-    expect(page).not_to have_css "span form"
-    expect(page).to have_css "div.in-favor > form"
-    expect(page).to have_css "div.against > form"
+    expect(page).to have_valid_html
   end
 
   describe "aria-pressed and method attributes" do
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 00577c2077..696467c05e 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -20,6 +20,35 @@ require "capybara/rails"
 require "capybara/rspec"
 require "selenium/webdriver"
 require "view_component/test_helpers"
+include PageValidations
+HTMLValidation.ignored_errors = ["missing <!DOCTYPE> declaration", "inserting missing 'title' element"]
+
+module PageValidations
+  class HaveValidHTML
+    alias_method :original_matches?, :matches?
+
+    def matches?(page)
+      begin
+        original_matches?(page)
+      rescue Errno::ENOENT => e
+        warn "WARNING: `tidy` not found; skipping HTML validation. " \
+          "Install the `tidy` HTML syntax checker in order to properly run this test."
+        true
+      end
+    end
+  end
+end
+
+class Capybara::Node::Simple
+  def current_url
+    "/"
+  end
+
+  def html
+    native.inner_html
+  end
+  alias_method :body, :html
+end
 
 module ViewComponent
   module TestHelpers

spec/components/comments/votes_component_spec.rb

We forgot to change the `span` tag when we replaced links with buttons in commit ba0d21b.

javierm added Bug Accessibility labels Mar 4, 2024

javierm self-assigned this Mar 4, 2024

javierm added this to Reviewing in Consul Democracy Mar 4, 2024

github-actions bot reviewed Mar 4, 2024

View reviewed changes

spec/components/comments/votes_component_spec.rb Outdated Show resolved Hide resolved

github-actions bot reviewed Mar 4, 2024

View reviewed changes

spec/components/comments/votes_component_spec.rb Outdated Show resolved Hide resolved

github-actions bot reviewed Mar 4, 2024

View reviewed changes

spec/components/comments/votes_component_spec.rb Outdated Show resolved Hide resolved

Fix invalid HTML in comment votes

a101608

We forgot to change the `span` tag when we replaced links with buttons in commit ba0d21b.

javierm force-pushed the fix_comment_votes_html branch from 0841ff2 to a101608 Compare March 4, 2024 23:42

taitus added the 2.1.1 label Mar 8, 2024

taitus self-assigned this Mar 18, 2024

taitus approved these changes Mar 18, 2024

View reviewed changes

Consul Democracy automation moved this from Reviewing to Testing Mar 18, 2024

javierm changed the base branch from fix_polls_test to master March 18, 2024 14:01

javierm merged commit bbac39e into master Mar 18, 2024
12 of 13 checks passed

Consul Democracy automation moved this from Testing to Release 2.1.1 Mar 18, 2024

javierm deleted the fix_comment_votes_html branch March 18, 2024 14:02

javierm removed the 2.1.1 label Mar 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix invalid HTML in comment votes #5426

Fix invalid HTML in comment votes #5426

javierm commented Mar 4, 2024 •

edited

Loading

Fix invalid HTML in comment votes #5426

Fix invalid HTML in comment votes #5426

Conversation

javierm commented Mar 4, 2024 • edited Loading

References

Objectives

Notes

javierm commented Mar 4, 2024 •

edited

Loading