From 597a21eca9c518c7ccc72bb42e9c48fbdc83f02d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 21:43:17 +0200 Subject: [PATCH 1/5] Add alt attribute to map images We were using it on the map displayed on the sidebar, but we weren't using it in the other places were the map is rendered. --- app/views/budgets/groups/show.html.erb | 2 +- app/views/proposals/map.html.erb | 2 +- spec/system/admin/site_customization/images_spec.rb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/budgets/groups/show.html.erb b/app/views/budgets/groups/show.html.erb index f0687630a..5fca4acd6 100644 --- a/app/views/budgets/groups/show.html.erb +++ b/app/views/budgets/groups/show.html.erb @@ -23,6 +23,6 @@
- <%= image_tag(image_path_for("map.jpg")) %> + <%= image_tag(image_path_for("map.jpg"), alt: t("shared.tags_cloud.districts_list")) %>
diff --git a/app/views/proposals/map.html.erb b/app/views/proposals/map.html.erb index dde4a610f..261cd49ef 100644 --- a/app/views/proposals/map.html.erb +++ b/app/views/proposals/map.html.erb @@ -13,7 +13,7 @@
- <%= image_tag(image_path_for("map.jpg"), usemap: "#map") %> + <%= image_tag(image_path_for("map.jpg"), alt: t("shared.tags_cloud.districts_list"), usemap: "#map") %>
diff --git a/spec/system/admin/site_customization/images_spec.rb b/spec/system/admin/site_customization/images_spec.rb index 3ed98ffa6..10ceb3149 100644 --- a/spec/system/admin/site_customization/images_spec.rb +++ b/spec/system/admin/site_customization/images_spec.rb @@ -45,19 +45,19 @@ describe "Admin custom images", :admin do visit proposals_path within("#map") do - expect(page).to have_css("img[src*='custom_map.jpg']") + expect(page).to have_css("img[src*='custom_map.jpg'][alt='Districts list']") end visit map_proposals_path within(".show-for-medium") do - expect(page).to have_css("img[src*='custom_map.jpg']") + expect(page).to have_css("img[src*='custom_map.jpg'][alt='Districts list']") end visit budget_group_path(budget, group) within(".show-for-medium") do - expect(page).to have_css("img[src*='custom_map.jpg']") + expect(page).to have_css("img[src*='custom_map.jpg'][alt='Districts list']") end end From 6d6c067296a4bd1d221c15074b286f39caee953a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 22:15:45 +0200 Subject: [PATCH 2/5] Use an empty alt attribute for decorative images This way people using screen readers will know that the image is a decorative one. --- app/views/dashboard/mailer/forward.html.erb | 4 ++-- app/views/dashboard/mailing/index.html.erb | 4 ++-- app/views/dashboard/poster/index.html.erb | 6 +++--- app/views/dashboard/poster/index.pdf.erb | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/views/dashboard/mailer/forward.html.erb b/app/views/dashboard/mailer/forward.html.erb index 553ce766a..3f85b7850 100644 --- a/app/views/dashboard/mailer/forward.html.erb +++ b/app/views/dashboard/mailer/forward.html.erb @@ -5,9 +5,9 @@ - <%= image_tag "quote_before_white.png", style: "max-width: 40px; vertical-align: top;" %> + <%= image_tag "quote_before_white.png", alt: "", style: "max-width: 40px; vertical-align: top;" %>

<%= @proposal.title %>

- <%= image_tag "quote_after_white.png", style: "max-width: 40px; vertical-align: top;" %> + <%= image_tag "quote_after_white.png", alt: "", style: "max-width: 40px; vertical-align: top;" %>

<%= sanitize(t("dashboard.mailer.forward.subtitle")) %>

diff --git a/app/views/dashboard/mailing/index.html.erb b/app/views/dashboard/mailing/index.html.erb index 713f85935..0f3c6c44b 100644 --- a/app/views/dashboard/mailing/index.html.erb +++ b/app/views/dashboard/mailing/index.html.erb @@ -2,9 +2,9 @@
- <%= image_tag "quote_before_white.png" %> + <%= image_tag "quote_before_white.png", alt: "" %>

<%= proposal.title %>

- <%= image_tag "quote_after_white.png" %> + <%= image_tag "quote_after_white.png", alt: "" %>

<%= sanitize(t("dashboard.mailer.forward.subtitle")) %>

diff --git a/app/views/dashboard/poster/index.html.erb b/app/views/dashboard/poster/index.html.erb index bbd988356..e8e7112cd 100644 --- a/app/views/dashboard/poster/index.html.erb +++ b/app/views/dashboard/poster/index.html.erb @@ -6,7 +6,7 @@

<%= t("dashboard.poster.index.poster_title") %>
- <%= image_tag("finger.png") %> + <%= image_tag("finger.png", alt: "") %> <%= t("dashboard.poster.index.poster_subtitle") %>

@@ -25,9 +25,9 @@

<%= t("dashboard.poster.index.support") %>

- <%= image_tag "quote_before_blue.png" %> + <%= image_tag "quote_before_blue.png", alt: "" %>

<%= proposal.title %>

- <%= image_tag "quote_after_blue.png" %> + <%= image_tag "quote_after_blue.png", alt: "" %>

<%= sanitize(t("dashboard.poster.index.footer", link: proposal_url(proposal))) %>

diff --git a/app/views/dashboard/poster/index.pdf.erb b/app/views/dashboard/poster/index.pdf.erb index 7c4eaebe1..9a8b0e383 100644 --- a/app/views/dashboard/poster/index.pdf.erb +++ b/app/views/dashboard/poster/index.pdf.erb @@ -11,7 +11,7 @@

<%= t("dashboard.poster.index.poster_title") %>
- <%= wicked_pdf_image_tag("finger.png") %> + <%= wicked_pdf_image_tag("finger.png", alt: "") %> <%= t("dashboard.poster.index.poster_subtitle") %>

@@ -30,9 +30,9 @@

<%= t("dashboard.poster.index.support") %>

- <%= wicked_pdf_image_tag "quote_before_blue.png" %> + <%= wicked_pdf_image_tag "quote_before_blue.png", alt: "" %>

<%= proposal.title %>

- <%= wicked_pdf_image_tag "quote_after_blue.png" %> + <%= wicked_pdf_image_tag "quote_after_blue.png", alt: "" %>

<%= sanitize(t("dashboard.poster.index.footer", link: proposal_url(proposal))) %>

From a6e6a90befa365ed181b8df5515ca24d1761471f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 22:24:43 +0200 Subject: [PATCH 3/5] Add alt test to proposal images in mail --- app/views/dashboard/mailer/forward.html.erb | 8 ++++++-- app/views/dashboard/mailing/index.html.erb | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/views/dashboard/mailer/forward.html.erb b/app/views/dashboard/mailer/forward.html.erb index 3f85b7850..6d16ab1f9 100644 --- a/app/views/dashboard/mailer/forward.html.erb +++ b/app/views/dashboard/mailer/forward.html.erb @@ -15,9 +15,13 @@ <% if @proposal.image.present? %> - <%= image_tag @proposal.image.variant(:large), style: "width: 100%; box-shadow: -16px 61px 49px -19px rgba(0,0,0,0.1);" %> + <%= image_tag @proposal.image.variant(:large), + alt: @proposal.image.title.unicode_normalize, + style: "width: 100%; box-shadow: -16px 61px 49px -19px rgba(0,0,0,0.1);" %> <% else %> - <%= image_tag "default_mailing.jpg", style: "width: 100%; box-shadow: -16px 61px 49px -19px rgba(0,0,0,0.1);" %> + <%= image_tag "default_mailing.jpg", + alt: "", + style: "width: 100%; box-shadow: -16px 61px 49px -19px rgba(0,0,0,0.1);" %> <% end %>

<%= t("dashboard.mailer.forward.hi") %>

diff --git a/app/views/dashboard/mailing/index.html.erb b/app/views/dashboard/mailing/index.html.erb index 0f3c6c44b..1f6a5a2ea 100644 --- a/app/views/dashboard/mailing/index.html.erb +++ b/app/views/dashboard/mailing/index.html.erb @@ -9,7 +9,11 @@
- <%= image_tag(proposal.image&.variant(:large).presence || "default_mailing.jpg") %> + <% if proposal.image.present? %> + <%= image_tag(proposal.image.variant(:large), alt: proposal.image.title.unicode_normalize) %> + <% else %> + <%= image_tag("default_mailing.jpg", alt: "") %> + <% end %>
From 815a4078d50a45b72f6774df0c86d472f71a1cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 12 Oct 2024 22:52:37 +0200 Subject: [PATCH 4/5] Check the alt attribute in XSS tests Not doing so was causing a test to fail when checking that all rendered image contain an `alt` attribute. --- spec/system/xss_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/system/xss_spec.rb b/spec/system/xss_spec.rb index 32b1cb03c..0046e2bdd 100644 --- a/spec/system/xss_spec.rb +++ b/spec/system/xss_spec.rb @@ -170,7 +170,7 @@ describe "Cross-Site Scripting protection" do end scenario "legislation version body filters script tags but not header IDs nor tags like images" do - markdown = "# Title 1\nlink" + markdown = "# Title 1\nlinktext" version = create(:legislation_draft_version, :published, body: "#{markdown}#{attack_code}") visit legislation_process_draft_version_path(version.process, version) @@ -178,6 +178,6 @@ describe "Cross-Site Scripting protection" do expect(page.text).not_to be_empty expect(page).to have_css "h1#title-1", text: "Title 1" expect(page).to have_link "link", href: "https://domain.com/url" - expect(page).to have_css('img[src="/image.png"') + expect(page).to have_css('img[src="/image.png"]') end end From 360a181c1870f591f8ab5a2721ec97b1cda7b93d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 13 Oct 2024 00:15:33 +0200 Subject: [PATCH 5/5] Use aria-hidden when rendering SVG avatars It looks like not all screen readers identify SVG images with empty aria labels as a decorative image, as reported by the Axe accessibility engine. So we're using `aria-hidden` instead, since we don't want the text of the SVG to be read by screen readers. We're using `aria-hidden` instead of the `presentation` role for the reasons mentioned in commit 35659d441. --- app/components/shared/avatar_component.rb | 2 +- spec/components/shared/avatar_component_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/shared/avatar_component.rb b/app/components/shared/avatar_component.rb index 40ce21f9b..a8f310069 100644 --- a/app/components/shared/avatar_component.rb +++ b/app/components/shared/avatar_component.rb @@ -49,7 +49,7 @@ class Shared::AvatarComponent < ApplicationComponent width: size, height: size, role: "img", - "aria-label": "", + "aria-hidden": true, style: "background-color: #{background_color}", class: "initialjs-avatar #{options[:class]}".strip } diff --git a/spec/components/shared/avatar_component_spec.rb b/spec/components/shared/avatar_component_spec.rb index cc1d7b1e1..66a82c5c7 100644 --- a/spec/components/shared/avatar_component_spec.rb +++ b/spec/components/shared/avatar_component_spec.rb @@ -8,7 +8,7 @@ describe Shared::AvatarComponent do render_inline component expect(page).to have_css "svg", count: 1 - expect(page).to have_css "svg[role='img'][aria-label='']" + expect(page).to have_css "svg[role='img'][aria-hidden='true']" end it "shows the initial letter of the name" do