FIX: Add aria-label attribute to cooked hashtags (#22182)

This commit adds an aria-label attribute to cooked hashtags using
the post/chat message decorateCooked functionality. I have just used
the inner content of the hashtag (the tag/category/channel name) for
the label -- we can reexamine at some point if we want something
different like "Link to dev category" or something, but from what I
can tell things like Twitter don't even have aria-labels for hashtags
so the text would be read out directly.

This commit also refactors any ruby specs checking the HTML of hashtags
to use rspec-html-matchers which is far clearer than having to maintain
the HTML structure in a HEREDOC for comparison, and gives better spec
failures.

c.f. https://meta.discourse.org/t/hashtags-are-getting-a-makeover/248866/23?u=martin

app/assets/javascripts/discourse/app/instance-initializers/hashtag-post-decorations.js

@@ -1,5 +1,5 @@
 import { withPluginApi } from "discourse/lib/plugin-api";
-import { replaceHashtagIconPlaceholder } from "discourse/lib/hashtag-autocomplete";
+import { decorateHashtags } from "discourse/lib/hashtag-autocomplete";
 
 export default {
   after: "hashtag-css-generator",
@@ -10,13 +10,10 @@ export default {
 
     withPluginApi("0.8.7", (api) => {
       if (siteSettings.enable_experimental_hashtag_autocomplete) {
-        api.decorateCookedElement(
-          (post) => replaceHashtagIconPlaceholder(post, site),
-          {
-            onlyStream: true,
-            id: "hashtag-icons",
-          }
-        );
+        api.decorateCookedElement((post) => decorateHashtags(post, site), {
+          onlyStream: true,
+          id: "hashtag-icons",
+        });
       }
     });
   },

app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js

@@ -25,8 +25,9 @@ export function cleanUpHashtagTypeClasses() {
 export function getHashtagTypeClasses() {
   return hashtagTypeClasses;
 }
-export function replaceHashtagIconPlaceholder(element, site) {
+export function decorateHashtags(element, site) {
   element.querySelectorAll(".hashtag-cooked").forEach((hashtagEl) => {
+    // Replace the empty icon placeholder span with actual icon HTML.
     const iconPlaceholderEl = hashtagEl.querySelector(
       ".hashtag-icon-placeholder"
     );
@@ -41,6 +42,10 @@ export function replaceHashtagIconPlaceholder(element, site) {
         .trim();
       iconPlaceholderEl.replaceWith(domFromString(hashtagIconHTML)[0]);
     }
+
+    // Add an aria-label to the hashtag element so that screen readers
+    // can read the hashtag text.
+    hashtagEl.setAttribute("aria-label", `${hashtagEl.innerText.trim()}`);
   });
 }
 

plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js

@@ -1,5 +1,5 @@
 import { decorateGithubOneboxBody } from "discourse/instance-initializers/onebox-decorators";
-import { replaceHashtagIconPlaceholder } from "discourse/lib/hashtag-autocomplete";
+import { decorateHashtags } from "discourse/lib/hashtag-autocomplete";
 import { withPluginApi } from "discourse/lib/plugin-api";
 import highlightSyntax from "discourse/lib/highlight-syntax";
 import I18n from "I18n";
@@ -73,10 +73,9 @@ export default {
       }
     );
 
-    api.decorateChatMessage(
-      (element) => replaceHashtagIconPlaceholder(element, site),
-      { id: "hashtagIcons" }
-    );
+    api.decorateChatMessage((element) => decorateHashtags(element, site), {
+      id: "hashtagIcons",
+    });
   },
 
   _getScrollParent(node, maxParentSelector) {

plugins/chat/spec/lib/chat/channel_archive_service_spec.rb

@@ -194,9 +194,20 @@ describe Chat::ChannelArchiveService do
           subject.new(@channel_archive).execute
           expect(@channel_archive.reload.complete?).to eq(true)
           pm_topic = Topic.private_messages.last
-          expect(pm_topic.first_post.cooked).to include(
-            "<a class=\"hashtag-cooked\" href=\"#{channel.relative_url}\" data-type=\"channel\" data-slug=\"#{channel.slug}\" data-id=\"#{channel.id}\" data-ref=\"#{channel.slug}::channel\"><span class=\"hashtag-icon-placeholder\"></span><span>#{channel.title(user)}</span></a>",
-          )
+          expect(pm_topic.first_post.cooked).to have_tag(
+            "a",
+            with: {
+              class: "hashtag-cooked",
+              href: channel.relative_url,
+              "data-type": "channel",
+              "data-slug": channel.slug,
+              "data-id": channel.id,
+              "data-ref": "#{channel.slug}::channel",
+            },
+          ) do
+            with_tag("span", with: { class: "hashtag-icon-placeholder" })
+            with_tag("span", text: channel.title(user))
+          end
         end
       end
 

plugins/chat/spec/models/chat/message_spec.rb

@@ -241,7 +241,7 @@ describe Chat::Message do
       )
     end
 
-    it "supports hashtag-autocomplete plugin" do
+    it "supports hashtag autocomplete" do
       SiteSetting.chat_enabled = true
       SiteSetting.enable_experimental_hashtag_autocomplete = true
 
@@ -250,9 +250,18 @@ describe Chat::Message do
 
       cooked = described_class.cook("##{category.slug}", user_id: user.id)
 
-      expect(cooked).to eq(
-        "<p><a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-slug=\"#{category.slug}\" data-id=\"#{category.id}\"><span class=\"hashtag-icon-placeholder\"></span><span>#{category.name}</span></a></p>",
-      )
+      expect(cooked).to have_tag(
+        "a",
+        with: {
+          class: "hashtag-cooked",
+          href: category.url,
+          "data-type": "category",
+          "data-slug": category.slug,
+          "data-id": category.id,
+        },
+      ) do
+        with_tag("span", with: { class: "hashtag-icon-placeholder" })
+      end
     end
 
     it "supports censored plugin" do

plugins/chat/spec/system/hashtag_autocomplete_spec.rb

@@ -68,15 +68,63 @@ describe "Using #hashtag autocompletion to search for and lookup channels", type
 
     cooked_hashtags = page.all(".hashtag-cooked", count: 3)
 
-    expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp)
-    <a class=\"hashtag-cooked\" href=\"#{channel2.relative_url}\" data-type=\"channel\" data-slug=\"random\" data-id=\"#{channel2.id}\"><svg class=\"fa d-icon d-icon-comment svg-icon hashtag-color--channel-#{channel2.id} svg-string\" xmlns=\"http://www.w3.org/2000/svg\"><use href=\"#comment\"></use></svg><span>Random</span></a>
-    HTML
-    expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp)
-    <a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-slug=\"raspberry-beret\" data-id="#{category.id}"><span class=\"hashtag-category-badge hashtag-color--category-#{category.id}\"></span><span>Raspberry</span></a>
-    HTML
-    expect(cooked_hashtags[2]["outerHTML"]).to eq(<<~HTML.chomp)
-    <a class=\"hashtag-cooked\" href=\"#{tag.url}\" data-type=\"tag\" data-slug=\"razed\" data-id="#{tag.id}"><svg class=\"fa d-icon d-icon-tag svg-icon hashtag-color--tag-#{tag.id} svg-string\" xmlns=\"http://www.w3.org/2000/svg\"><use href=\"#tag\"></use></svg><span>razed</span></a>
-    HTML
+    expect(cooked_hashtags[0]["outerHTML"]).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: channel2.relative_url,
+        "data-type": "channel",
+        "data-slug": "random",
+        "data-id": channel2.id,
+        "aria-label": "Random",
+      },
+    ) do
+      with_tag(
+        "svg",
+        with: {
+          class:
+            "fa d-icon d-icon-comment svg-icon hashtag-color--channel-#{channel2.id} svg-string",
+        },
+      ) { with_tag("use", with: { href: "#comment" }) }
+    end
+
+    expect(cooked_hashtags[1]["outerHTML"]).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: category.url,
+        "data-type": "category",
+        "data-slug": "raspberry-beret",
+        "data-id": category.id,
+        "aria-label": "Raspberry",
+      },
+    ) do
+      with_tag(
+        "span",
+        with: {
+          class: "hashtag-category-badge hashtag-color--category-#{category.id}",
+        },
+      )
+    end
+
+    expect(cooked_hashtags[2]["outerHTML"]).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: tag.url,
+        "data-type": "tag",
+        "data-slug": "razed",
+        "data-id": tag.id,
+        "aria-label": "razed",
+      },
+    ) do
+      with_tag(
+        "svg",
+        with: {
+          class: "fa d-icon d-icon-tag svg-icon hashtag-color--tag-#{tag.id} svg-string",
+        },
+      ) { with_tag("use", with: { href: "#tag" }) }
+    end
   end
 
   context "when a user cannot access the category for a cooked channel hashtag" do

spec/lib/email/styles_spec.rb

@@ -109,12 +109,10 @@ RSpec.describe Email::Styles do
     end
 
     it "replaces hashtag-cooked text with raw #hashtag" do
-      hashtag_html =
-        "<a class=\"hashtag-cooked\" href=\"#{Discourse.base_url}/c/123/dev\" data-type=\"category\" data-slug=\"dev\"><svg class=\"fa d-icon d-icon-folder svg-icon svg-node\"><use href=\"#folder\"></use></svg><span>Dev Zone</span></a>"
-      frag = html_fragment(hashtag_html)
-      expect(frag.at("a").text.chomp).to eq("#dev")
-      hashtag_html =
-        "<a class=\"hashtag-cooked\" href=\"#{Discourse.base_url}/c/123/dev\" data-type=\"category\" data-slug=\"dev\"><svg class=\"fa d-icon d-icon-folder svg-icon svg-node\">Dev Zone</a>"
+      category = Fabricate(:category, name: "dev", slug: "dev")
+      post = Fabricate(:post, raw: "this is #dev")
+      post.rebake!
+      hashtag_html = post.cooked
       frag = html_fragment(hashtag_html)
       expect(frag.at("a").text.chomp).to eq("#dev")
     end

spec/lib/oneboxer_spec.rb

@@ -190,9 +190,31 @@ RSpec.describe Oneboxer do
           .inner_html
           .chomp
           .strip
-      expect(preview).to eq(<<~HTML.chomp.strip)
-        This post has some hashtags, <a class="hashtag-cooked" href="#{category.url}" data-type="category" data-slug="random" data-id="#{category.id}"><span class="hashtag-icon-placeholder"></span>#{category.name}</a> and <a class="hashtag-cooked" href="#{tag.url}" data-type="tag" data-slug="bug" data-id="#{tag.id}"><span class="hashtag-icon-placeholder"></span>#{tag.name}</a>
-      HTML
+      expect(preview).to include("This post has some hashtags")
+      expect(preview).to have_tag(
+        "a",
+        with: {
+          class: "hashtag-cooked",
+          href: category.url,
+          "data-type": "category",
+          "data-slug": category.slug,
+          "data-id": category.id,
+        },
+      ) do
+        with_tag("span", with: { class: "hashtag-icon-placeholder" })
+      end
+      expect(preview).to have_tag(
+        "a",
+        with: {
+          class: "hashtag-cooked",
+          href: tag.url,
+          "data-type": "tag",
+          "data-slug": tag.name,
+          "data-id": tag.id,
+        },
+      ) do
+        with_tag("span", with: { class: "hashtag-icon-placeholder" })
+      end
     end
   end
 

spec/lib/pretty_text_spec.rb

@@ -1776,24 +1776,60 @@ RSpec.describe PrettyText do
 
     cooked = PrettyText.cook(" #unknown::tag #known #known::tag #testing #secret", user_id: user.id)
 
-    expect(cooked).to include("<span class=\"hashtag-raw\">#unknown::tag</span>")
-    expect(cooked).to include(
-      "<a class=\"hashtag-cooked\" href=\"#{category2.url}\" data-type=\"category\" data-slug=\"known\" data-id=\"#{category2.id}\"><span class=\"hashtag-icon-placeholder\"></span><span>known</span></a>",
-    )
-    expect(cooked).to include(
-      "<a class=\"hashtag-cooked\" href=\"/tag/known\" data-type=\"tag\" data-slug=\"known\" data-id=\"#{tag.id}\" data-ref=\"known::tag\"><span class=\"hashtag-icon-placeholder\"></span><span>known</span></a>",
-    )
-    expect(cooked).to include(
-      "<a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-slug=\"testing\" data-id=\"#{category.id}\"><span class=\"hashtag-icon-placeholder\"></span><span>testing</span></a>",
-    )
-    expect(cooked).to include("<span class=\"hashtag-raw\">#secret</span>")
+    expect(cooked).to have_tag("span", text: "#unknown::tag", with: { class: "hashtag-raw" })
+    expect(cooked).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: category2.url,
+        "data-type": "category",
+        "data-slug": category2.slug,
+        "data-id": category2.id,
+      },
+    ) do
+      with_tag("span", with: { class: "hashtag-icon-placeholder" })
+    end
+    expect(cooked).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: category.url,
+        "data-type": "category",
+        "data-slug": category.slug,
+        "data-id": category.id,
+      },
+    ) do
+      with_tag("span", with: { class: "hashtag-icon-placeholder" })
+    end
+    expect(cooked).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: tag.url,
+        "data-type": "tag",
+        "data-slug": tag.name,
+        "data-id": tag.id,
+      },
+    ) do
+      with_tag("span", with: { class: "hashtag-icon-placeholder" })
+    end
+    expect(cooked).to have_tag("span", text: "#secret", with: { class: "hashtag-raw" })
 
     # If the user hash access to the private category it should be cooked with the details + icon
     group.add(user)
     cooked = PrettyText.cook(" #unknown::tag #known #known::tag #testing #secret", user_id: user.id)
-    expect(cooked).to include(
-      "<a class=\"hashtag-cooked\" href=\"#{private_category.url}\" data-type=\"category\" data-slug=\"secret\" data-id=\"#{private_category.id}\"><span class=\"hashtag-icon-placeholder\"></span><span>secret</span></a>",
-    )
+    expect(cooked).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: private_category.url,
+        "data-type": "category",
+        "data-slug": private_category.slug,
+        "data-id": private_category.id,
+      },
+    ) do
+      with_tag("span", with: { class: "hashtag-icon-placeholder" })
+    end
 
     cooked = PrettyText.cook("[`a` #known::tag here](http://example.com)", user_id: user.id)
 
@@ -1809,10 +1845,18 @@ RSpec.describe PrettyText do
     expect(cooked).to eq(html.strip)
 
     cooked = PrettyText.cook("<A href='/a'>test</A> #known::tag", user_id: user.id)
-    html = <<~HTML
-      <p><a href="/a">test</a> <a class="hashtag-cooked" href="/tag/known" data-type="tag" data-slug="known" data-id=\"#{tag.id}\" data-ref="known::tag"><span class=\"hashtag-icon-placeholder\"></span><span>known</span></a></p>
-    HTML
-    expect(cooked).to eq(html.strip)
+    expect(cooked).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: tag.url,
+        "data-type": "tag",
+        "data-slug": tag.name,
+        "data-id": tag.id,
+      },
+    ) do
+      with_tag("span", with: { class: "hashtag-icon-placeholder" })
+    end
 
     # ensure it does not fight with the autolinker
     expect(PrettyText.cook(" http://somewhere.com/#known")).not_to include("hashtag")
@@ -2051,22 +2095,35 @@ HTML
         replacement: "discourse",
       )
 
-      expect(PrettyText.cook("@test #test test")).to match_html(<<~HTML)
-        <p>
-          <a class="mention" href="/u/test">@test</a>
-          <a class="hashtag" href="/c/test/#{category.id}">#<span>test</span></a>
-          tdiscourset
-        </p>
-      HTML
+      cooked = PrettyText.cook("@test #test test")
+      expect(cooked).to have_tag("a", text: "@test", with: { class: "mention", href: "/u/test" })
+      expect(cooked).to have_tag(
+        "a",
+        text: "#test",
+        with: {
+          class: "hashtag",
+          href: "/c/test/#{category.id}",
+        },
+      )
+      expect(cooked).to include("tdiscourset")
 
       SiteSetting.enable_experimental_hashtag_autocomplete = true
-      expect(PrettyText.cook("@test #test test")).to match_html(<<~HTML)
-        <p>
-          <a class="mention" href="/u/test">@test</a>
-          <a class="hashtag-cooked" href="#{category.url}" data-type="category" data-slug="test" data-id="#{category.id}"><span class="hashtag-icon-placeholder"></span><span>test</span></a>
-          tdiscourset
-        </p>
-      HTML
+      cooked = PrettyText.cook("@test #test test")
+      expect(cooked).to have_tag("a", text: "@test", with: { class: "mention", href: "/u/test" })
+      expect(cooked).to have_tag(
+        "a",
+        text: "test",
+        with: {
+          class: "hashtag-cooked",
+          href: "/c/test/#{category.id}",
+          "data-type": "category",
+          "data-slug": category.slug,
+          "data-id": category.id,
+        },
+      ) do
+        with_tag("span", with: { class: "hashtag-icon-placeholder" })
+      end
+      expect(cooked).to include("tdiscourset")
     end
 
     it "supports overlapping words" do

spec/system/hashtag_autocomplete_spec.rb

@@ -53,25 +53,53 @@ describe "Using #hashtag autocompletion to search for and lookup categories and
     )
   end
 
-  it "cooks the selected hashtag clientside with the correct url and icon" do
+  it "cooks the selected hashtag clientside in the composer preview with the correct url and icon" do
     visit_topic_and_initiate_autocomplete
     hashtag_results = page.all(".hashtag-autocomplete__link", count: 2)
     hashtag_results[0].click
     expect(page).to have_css(".hashtag-cooked")
     cooked_hashtag = page.find(".hashtag-cooked")
-    expected = <<~HTML.chomp
-      <a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-id=\"#{category.id}\" data-slug=\"cool-cat\" tabindex=\"-1\"><span class="hashtag-category-badge hashtag-color--category-#{category.id}"></span><span>Cool Category</span></a>
-    HTML
-    expect(cooked_hashtag["outerHTML"].squish).to eq(expected)
+
+    expect(cooked_hashtag["outerHTML"]).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: category.url,
+        "data-type": "category",
+        "data-slug": category.slug,
+        "data-id": category.id,
+      },
+    ) do
+      with_tag(
+        "span",
+        with: {
+          class: "hashtag-category-badge hashtag-color--category-#{category.id}",
+        },
+      )
+    end
 
     visit_topic_and_initiate_autocomplete
     hashtag_results = page.all(".hashtag-autocomplete__link", count: 2)
     hashtag_results[1].click
     expect(page).to have_css(".hashtag-cooked")
     cooked_hashtag = page.find(".hashtag-cooked")
-    expect(cooked_hashtag["outerHTML"].squish).to eq(<<~HTML.chomp)
-      <a class=\"hashtag-cooked\" href=\"#{tag.url}\" data-type=\"tag\" data-id=\"#{tag.id}\" data-slug=\"cooltag\" tabindex=\"-1\"><svg class=\"fa d-icon d-icon-tag svg-icon hashtag-color--tag-#{tag.id} svg-string\" xmlns=\"http://www.w3.org/2000/svg\"><use href=\"#tag\"></use></svg><span>cooltag</span></a>
-      HTML
+    expect(cooked_hashtag["outerHTML"]).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: tag.url,
+        "data-type": "tag",
+        "data-slug": tag.name,
+        "data-id": tag.id,
+      },
+    ) do
+      with_tag(
+        "svg",
+        with: {
+          class: "fa d-icon d-icon-tag svg-icon hashtag-color--tag-#{tag.id} svg-string",
+        },
+      ) { with_tag("use", with: { href: "#tag" }) }
+    end
   end
 
   it "cooks the hashtags for tag and category correctly serverside when the post is saved to the database" do
@@ -85,12 +113,43 @@ describe "Using #hashtag autocompletion to search for and lookup categories and
 
     cooked_hashtags = page.all(".hashtag-cooked", count: 2)
 
-    expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp)
-    <a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-slug=\"cool-cat\" data-id=\"#{category.id}\"><span class=\"hashtag-category-badge hashtag-color--category-#{category.id}\"></span><span>Cool Category</span></a>
-    HTML
-    expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp)
-    <a class=\"hashtag-cooked\" href=\"#{tag.url}\" data-type=\"tag\" data-slug=\"cooltag\" data-id=\"#{tag.id}\"><svg class=\"fa d-icon d-icon-tag svg-icon hashtag-color--tag-#{tag.id} svg-string\" xmlns=\"http://www.w3.org/2000/svg\"><use href=\"#tag\"></use></svg><span>cooltag</span></a>
-    HTML
+    expect(cooked_hashtags[0]["outerHTML"]).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: category.url,
+        "data-type": "category",
+        "data-slug": category.slug,
+        "data-id": category.id,
+        "aria-label": category.name,
+      },
+    ) do
+      with_tag(
+        "span",
+        with: {
+          class: "hashtag-category-badge hashtag-color--category-#{category.id}",
+        },
+      )
+    end
+
+    expect(cooked_hashtags[1]["outerHTML"]).to have_tag(
+      "a",
+      with: {
+        class: "hashtag-cooked",
+        href: tag.url,
+        "data-type": "tag",
+        "data-slug": tag.name,
+        "data-id": tag.id,
+        "aria-label": tag.name,
+      },
+    ) do
+      with_tag(
+        "svg",
+        with: {
+          class: "fa d-icon d-icon-tag svg-icon hashtag-color--tag-#{tag.id} svg-string",
+        },
+      ) { with_tag("use", with: { href: "#tag" }) }
+    end
   end
 
   context "when a user cannot access the category for a hashtag cooked in another post" do