From b6857348aadc38dcffd73e632d37b0af3f9e5d5a Mon Sep 17 00:00:00 2001
From: Michael Prentice <splaktar@gmail.com>
Date: Mon, 21 Sep 2020 18:01:44 -0400
Subject: [PATCH] fix(icon): providing empty alt or aria-label attributes do
 not hide them from a11y

- change behavior around how `aria-label` and `aria-hidden` is applied
  to match documentation
- clarify documentation around `alt` and `aria-label` behavior
- fix/improve Closure types
- remove unused variable and out of date comments
- replace blacklist with block-list in comments

Fixes #10721
---
 .../demoFontIconsWithClassnames/index.html    | 18 ++---
 .../demoFontIconsWithClassnames/script.js     |  3 -
 src/components/icon/js/iconDirective.js       | 68 ++++++++++---------
 src/core/services/aria/aria.js                | 14 ++--
 4 files changed, 53 insertions(+), 50 deletions(-)

diff --git a/src/components/icon/demoFontIconsWithClassnames/index.html b/src/components/icon/demoFontIconsWithClassnames/index.html
index 0bc4621a22..6032f24b13 100644
--- a/src/components/icon/demoFontIconsWithClassnames/index.html
+++ b/src/components/icon/demoFontIconsWithClassnames/index.html
@@ -8,11 +8,11 @@
         <md-icon md-font-icon="{{ font.name }}"
             ng-style="{color: !font.theme && font.color, 'font-size': it.size + 'px', height: it.size + 'px'}"
             ng-class="font.theme"
-            aria-label="{{ font.name + it.size }}"
-            class="step "></md-icon>
+            aria-label="{{ font.name + '-' + it.size }}"
+            class="step"></md-icon>
       </div>
       <div class="preview-scale">
-        <span class="step" style="padding-left:{{ it.padding }}px">{{ it.size }}px</span>
+        <span class="step" style="{{ 'padding-left: ' + it.padding + 'px'}}">{{ it.size }}px</span>
       </div>
     </div>
   </div>
@@ -20,16 +20,16 @@
   <!-- For this demo, include the font-faces needed by mdIcon above -->
   <style>
     @font-face {
-      font-family:"icomoon";
-      src:url("https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.eot");
-      font-weight:normal;
-      font-style:normal;
+      font-family: "icomoon";
+      src: url("https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.eot");
+      font-weight: normal;
+      font-style: normal;
     }
 
     @font-face {
       font-family: 'icomoon';
-      src:url('https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.eot?-cmq1um');
-      src:url('https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.eot?#iefix-cmq1um') format('embedded-opentype'),
+      src: url('https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.eot?-cmq1um');
+      src: url('https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.eot?#iefix-cmq1um') format('embedded-opentype'),
       url('https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.woff?-cmq1um') format('woff'),
       url('https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.ttf?-cmq1um') format('truetype'),
       url('https://cdn.rawgit.com/angular/material/master/docs/app/fonts/icomoon.svg?-cmq1um#icomoon') format('svg');
diff --git a/src/components/icon/demoFontIconsWithClassnames/script.js b/src/components/icon/demoFontIconsWithClassnames/script.js
index d1563fe23e..b8f1ea9191 100644
--- a/src/components/icon/demoFontIconsWithClassnames/script.js
+++ b/src/components/icon/demoFontIconsWithClassnames/script.js
@@ -21,9 +21,6 @@ angular
       ];
 
       $scope.fonts = [].concat(iconData);
-
-
-
   })
   .config(function($mdThemingProvider){
     // Update the theme colors to use themes on font-icons
diff --git a/src/components/icon/js/iconDirective.js b/src/components/icon/js/iconDirective.js
index 266c90a21e..07ae7c063a 100644
--- a/src/components/icon/js/iconDirective.js
+++ b/src/components/icon/js/iconDirective.js
@@ -105,22 +105,25 @@ angular
  * icon and to change the icon size. These settings only affect the downloaded icons.
  *
  * @param {string} md-font-icon String name of CSS icon associated with the font-face will be used
- * to render the icon. Requires the fonts and the named CSS styles to be preloaded.
+ *  to render the icon. Requires the fonts and the named CSS styles to be preloaded.
  * @param {string} md-font-set CSS style name associated with the font library; which will be assigned as
- * the class for the font-icon ligature. This value may also be an alias that is used to lookup the classname;
- * internally use `$mdIconProvider.fontSet(<alias>)` to determine the style name.
+ *  the class for the font-icon ligature. This value may also be an alias that is used to lookup the classname;
+ *  internally use `$mdIconProvider.fontSet(<alias>)` to determine the style name.
  * @param {string} md-svg-src String URL (or expression) used to load, cache, and display an
- *     external SVG.
+ *  external SVG.
  * @param {string} md-svg-icon md-svg-icon String name used for lookup of the icon from the internal cache;
- *     interpolated strings or expressions may also be used. Specific set names can be used with
- *     the syntax `<set name>:<icon name>`.<br/><br/>
- * To use icon sets, developers are required to pre-register the sets using the `$mdIconProvider` service.
- * @param {string=} aria-label Labels icon for accessibility. If an empty string is provided, icon
- * will be hidden from accessibility layer with `aria-hidden="true"`. If there's no aria-label on the icon
- * nor a label on the parent element, a warning will be logged to the console.
- * @param {string=} alt Labels icon for accessibility. If an empty string is provided, icon
- * will be hidden from accessibility layer with `aria-hidden="true"`. If there's no alt on the icon
- * nor a label on the parent element, a warning will be logged to the console.
+ *  interpolated strings or expressions may also be used. Specific set names can be used with
+ *  the syntax `<set name>:<icon name>`.<br/><br/>
+ *  To use icon sets, developers are required to pre-register the sets using the `$mdIconProvider` service.
+ * @param {string=} aria-label Labels the icon for accessibility. If an empty string is provided,
+ *  the icon will be hidden from the accessibility layer with `aria-hidden="true"`. If there is no
+ *  `aria-label` attribute on the icon, we check the following, in order: the `alt` attribute, the
+ *  `aria-label` from the parent element, the icon's `md-font-icon` or `md-svg-icon` string, and the
+ *  text content inside `<md-icon></md-icon>`. If none of these have any text, the icon is hidden
+ *  from the accessibility layer with `aria-hidden="true"`.
+ * @param {string=} alt Labels the icon for accessibility. If an empty string is provided and the
+ *  icon has no `aria-label`, then the icon will be hidden from accessibility layer with
+ *  `aria-hidden="true"`.
  *
  * @usage
  * When using SVGs:
@@ -198,7 +201,10 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria, $sce) {
 
   /**
    * Directive postLink
-   * Supports embedded SVGs, font-icons, & external SVGs
+   * Supports embedded SVGs, font-icons, & external SVGs.
+   * @param {IScope} scope
+   * @param {JQLite} element
+   * @param {IAttributes} attr
    */
   function postLink(scope, element, attr) {
     $mdTheming(element);
@@ -210,15 +216,6 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria, $sce) {
     attr.$observe('mdFontIcon', fontIconChanged);
     attr.$observe('mdFontSet', fontIconChanged);
 
-    // Keep track of the content of the svg src so we can compare against it later to see if the
-    // attribute is static (and thus safe).
-    var originalSvgSrc = element[0].getAttribute(attr.$attr.mdSvgSrc);
-
-    // If using a font-icon, then the textual name of the icon itself
-    // provides the aria-label.
-
-    var attrName = attr.$normalize(attr.$attr.mdSvgIcon || attr.$attr.mdSvgSrc || '');
-
     /* Provide a default accessibility role of img */
     if (!attr.role) {
       $mdAria.expect(element, 'role', 'img');
@@ -226,24 +223,33 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria, $sce) {
       attr.role = 'img';
     }
 
+    // If the aria-label is explicitly set to the empty string, then hide this element from the
+    // accessibility layer.
+    if (element[0].hasAttribute('aria-label') && attr.ariaLabel === '') {
+      element.attr('aria-hidden', true);
+    }
+
     /* Don't process ARIA if already valid */
     if (attr.role === "img" && !attr.ariaHidden && !$mdAria.hasAriaLabel(element)) {
-      var iconName;
-      if (attr.alt) {
-        /* Use alt text by default if available */
+      // If the developer signals to hide this icon from the accessibility layer, do so.
+      if (element[0].hasAttribute('alt') && attr.alt === '') {
+        element.attr('aria-hidden', true);
+      } else if (attr.alt) {
+        /* Use the alt text for the aria-label by default, if available. */
         $mdAria.expect(element, 'aria-label', attr.alt);
       } else if ($mdAria.parentHasAriaLabel(element, 2)) {
-        /* Parent has ARIA so we will assume it will describe the image */
+        /* Parent has ARIA so we will assume it will describe the icon. */
         $mdAria.expect(element, 'aria-hidden', 'true');
-      } else if (iconName = (attr.mdFontIcon || attr.mdSvgIcon || element.text())) {
-        /* Use icon name as aria-label */
-        $mdAria.expect(element, 'aria-label', iconName);
+      } else if (attr.mdFontIcon || attr.mdSvgIcon || element.text()) {
+        /* Use icon name or node's text content as the aria-label */
+        $mdAria.expect(element, 'aria-label', attr.mdFontIcon || attr.mdSvgIcon || element.text());
       } else {
-        /* No label found */
+        /* No label found, hide this icon from the accessibility layer */
         $mdAria.expect(element, 'aria-hidden', 'true');
       }
     }
 
+    var attrName = attr.$normalize(attr.$attr.mdSvgIcon || attr.$attr.mdSvgSrc || '');
     if (attrName) {
       // Use either pre-configured SVG or URL source, respectively.
       attr.$observe(attrName, function(attrVal) {
diff --git a/src/core/services/aria/aria.js b/src/core/services/aria/aria.js
index a5a65196a8..7a14a3c6be 100644
--- a/src/core/services/aria/aria.js
+++ b/src/core/services/aria/aria.js
@@ -73,9 +73,9 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {
 
   /**
    * Check if expected attribute has been specified on the target element or child
-   * @param element
-   * @param attrName
-   * @param {optional} defaultValue What to set the attr to if no value is found
+   * @param {string|JQLite} element
+   * @param {string} attrName
+   * @param {string=} defaultValue What to set the attr to if no value is found
    */
   function expect(element, attrName, defaultValue) {
 
@@ -192,8 +192,8 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {
 
   /**
    * Check if expected element's parent has aria label attribute and has valid role and tagName
-   * @param element
-   * @param {optional} level Number of levels deep search should be performed
+   * @param {string|JQLite|Node & ParentNode} element
+   * @param {number=} level Number of levels deep search should be performed
    */
   function parentHasAriaLabel(element, level) {
     level = level || 1;
@@ -214,7 +214,7 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {
       if (!hasAriaLabel(parentNode)) {
         return false;
       }
-      /* Perform role blacklist check */
+      /* Perform role block-list check */
       if (parentNode.hasAttribute('role')) {
         switch (parentNode.getAttribute('role').toLowerCase()) {
           case 'command':
@@ -236,7 +236,7 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {
             return false;
         }
       }
-      /* Perform tagName blacklist check */
+      /* Perform tagName block-list check */
       switch (parentNode.tagName.toLowerCase()) {
         case 'abbr':
         case 'acronym':