Fix Font Awesome markup and CSS

Here is the patch with both CSS and markup fixes.
AXE has no issues with that: https://jsfiddle.net/s23w8q4j/

Fixing tests...

This patch #9 worked for me! Thank you so much!

Is `sr-only` a native Drupal CSS class? I don't know so asking. I think traditionally `visually-hidden` may be the more appropriate class to use here. So I'd take the patch in the related issue over this one.

Hello @Baysaa,

sr-only is a fontawesome-specific class, added since version 4.6.0 (commit)
I'm not a maintainer but I'm not against changing it to "visually-hidden" which would make it independent from any FA version.
Feel free to post a patch.

Path #9 worked for me as well. Thanks!

Thanks, could this please be turned into a MR for further review and merge?

Hi @Anybody, sure, done.

@herved if the icon's using FA then it makes sense that sr-only should also work, thanks!

Think visually hidden would be better as even though we have font awesome icons the library might not be present (so the checkbox setting is unchecked) so not sure sr-only would cover all cases.

smustgrave → closed merge request !13

I'm testing the latest dev of the 2.0.x branch, including this commit, and it's still outputting aria-label inappropriately.

To reproduce:

1. Install Drupal core 10.3.6 using the standard profile.
2. Require extlink at 2.0.x-dev via composer.
3. Log in as an administrator.
4. Enable the extlink module.
5. In the extlink settings, check the checkbox for "Use Font Awesome icons instead of images" and save the configuration.
6. Rebuild the site cache.
7. View the automatically-generated home page content of the standard profile, which includes external links.
8. Using the browser code inspector, view the code for the "Drupal community" link:

<a href="https://www.drupal.org/community" class="ext" data-extlink="" rel="noreferrer">Drupal 
  <span class="extlink-nobreak" aria-label="(link is external)">community
    <span class="fa-ext extlink" role="img" aria-hidden="false">
      <i class="fa fa-external-link" data-extlink-placement="append">
      </i>
      <span class="visually-hidden"></span>
    </span>
  </span>
</a>

First, the aria-label attribute is being output on a nested span tag. The attribute isn't intended for use on elements with a generic role, including the HTML div and span tags. Most assistive technologies will ignore aria-label on generic tags.

Second, the aria-hidden attribute doesn't need to be added if the value is "false." See aria-hidden where the Note says "At the time of this writing, aria-hidden="false" is known to work inconsistently in browsers. As future implementations improve, use caution and test thoroughly before relying on this approach." In the proposed draft for ARIA 1.3, the "false" value is supposed to be handled the same as if it's undefined (i.e. attribute is not present).

Third, the span containing the FA icon markup has a role of "img" and is not hidden from assistive technology, which means it should have something in the way of alt text, but the markup inside that span doesn't contain any text.

Indeed this doesn't seem valid: https://jsfiddle.net/cafsjowu/1/
It looks like 📌 Prevent extlink icons from breaking into new lines Needs work recently committed added an extra span wrapper in the mean time?
Can you please create a new issue @cboyden? thanks

PS: The added code there looks convoluted. I wonder, wouldn't simply   work?

I'll revert this when I'm back in front of my computer.

Reverted