From f129167895a3b59794e410f4d1adf8b03ef0313c Mon Sep 17 00:00:00 2001
From: marcysutton <holla@marcysutton.com>
Date: Thu, 2 Jul 2015 10:15:34 -0700
Subject: [PATCH] fix(ngAria): clean up tabindex usage

* Do not put tabindex on native controls using ng-model or ng-click
* Uses a single nodeBlacklist to limit which elements receive support

Closes #11500
---
 src/ngAria/aria.js      | 74 ++++++++++++++++++++++-------------------
 test/ngAria/ariaSpec.js | 47 ++++++++++++++------------
 2 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js
index c1038f2f26c7..e8c3bcbff6b2 100644
--- a/src/ngAria/aria.js
+++ b/src/ngAria/aria.js
@@ -52,6 +52,16 @@
 var ngAriaModule = angular.module('ngAria', ['ng']).
                         provider('$aria', $AriaProvider);
 
+/**
+* Internal Utilities 
+*/
+var nodeBlackList = ['BUTTON', 'A', 'INPUT', 'TEXTAREA', 'SELECT', 'DETAILS', 'SUMMARY'];
+
+var isNodeOneOf = function (elem, nodeTypeArray) {
+  if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
+    return true;
+  }
+}
 /**
  * @ngdoc provider
  * @name $ariaProvider
@@ -113,10 +123,10 @@ function $AriaProvider() {
     config = angular.extend(config, newConfig);
   };
 
-  function watchExpr(attrName, ariaAttr, negate) {
+  function watchExpr(attrName, ariaAttr, nodeBlackList, negate) {
     return function(scope, elem, attr) {
       var ariaCamelName = attr.$normalize(ariaAttr);
-      if (config[ariaCamelName] && !attr[ariaCamelName]) {
+      if (config[ariaCamelName] && !isNodeOneOf(elem, nodeBlackList) && !attr[ariaCamelName]) {
         scope.$watch(attr[attrName], function(boolVal) {
           // ensure boolean value
           boolVal = negate ? !boolVal : !!boolVal;
@@ -125,7 +135,6 @@ function $AriaProvider() {
       }
     };
   }
-
   /**
    * @ngdoc service
    * @name $aria
@@ -184,10 +193,10 @@ function $AriaProvider() {
 
 
 ngAriaModule.directive('ngShow', ['$aria', function($aria) {
-  return $aria.$$watchExpr('ngShow', 'aria-hidden', true);
+  return $aria.$$watchExpr('ngShow', 'aria-hidden', [], true);
 }])
 .directive('ngHide', ['$aria', function($aria) {
-  return $aria.$$watchExpr('ngHide', 'aria-hidden', false);
+  return $aria.$$watchExpr('ngHide', 'aria-hidden', [], false);
 }])
 .directive('ngModel', ['$aria', function($aria) {
 
@@ -261,6 +270,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
                 scope.$watch(ngAriaWatchModelValue, shape === 'radio' ?
                     getRadioReaction() : ngAriaCheckboxReaction);
               }
+              if (needsTabIndex) {
+                elem.attr('tabindex', 0);
+              }
               break;
             case 'range':
               if (shouldAttachRole(shape, elem)) {
@@ -289,6 +301,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
                   });
                 }
               }
+              if (needsTabIndex) {
+                elem.attr('tabindex', 0);
+              }
               break;
             case 'multiline':
               if (shouldAttachAttr('aria-multiline', 'ariaMultiline', elem)) {
@@ -297,10 +312,6 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
               break;
           }
 
-          if (needsTabIndex) {
-            elem.attr('tabindex', 0);
-          }
-
           if (ngModel.$validators.required && shouldAttachAttr('aria-required', 'ariaRequired', elem)) {
             scope.$watch(function ngAriaRequiredWatch() {
               return ngModel.$error.required;
@@ -322,7 +333,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
   };
 }])
 .directive('ngDisabled', ['$aria', function($aria) {
-  return $aria.$$watchExpr('ngDisabled', 'aria-disabled');
+  return $aria.$$watchExpr('ngDisabled', 'aria-disabled', []);
 }])
 .directive('ngMessages', function() {
   return {
@@ -342,35 +353,28 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
       var fn = $parse(attr.ngClick, /* interceptorFn */ null, /* expensiveChecks */ true);
       return function(scope, elem, attr) {
 
-        var nodeBlackList = ['BUTTON', 'A', 'INPUT', 'TEXTAREA'];
+        if (!isNodeOneOf(elem, nodeBlackList)) {
 
-        function isNodeOneOf(elem, nodeTypeArray) {
-          if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
-            return true;
+          if ($aria.config('bindRoleForClick') && !elem.attr('role')) {
+            elem.attr('role', 'button');
           }
-        }
 
-        if ($aria.config('bindRoleForClick')
-            && !elem.attr('role')
-              && !isNodeOneOf(elem, nodeBlackList)) {
-          elem.attr('role', 'button');
-        }
-
-        if ($aria.config('tabindex') && !elem.attr('tabindex')) {
-          elem.attr('tabindex', 0);
-        }
+          if ($aria.config('tabindex') && !elem.attr('tabindex')) {
+            elem.attr('tabindex', 0);
+          }
 
-        if ($aria.config('bindKeypress') && !attr.ngKeypress && !isNodeOneOf(elem, nodeBlackList)) {
-          elem.on('keypress', function(event) {
-            var keyCode = event.which || event.keyCode;
-            if (keyCode === 32 || keyCode === 13) {
-              scope.$apply(callback);
-            }
+          if ($aria.config('bindKeypress') && !attr.ngKeypress) {
+            elem.on('keypress', function(event) {
+              var keyCode = event.which || event.keyCode;
+              if (keyCode === 32 || keyCode === 13) {
+                scope.$apply(callback);
+              }
 
-            function callback() {
-              fn(scope, { $event: event });
-            }
-          });
+              function callback() {
+                fn(scope, { $event: event });
+              }
+            });
+          }
         }
       };
     }
@@ -378,7 +382,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
 }])
 .directive('ngDblclick', ['$aria', function($aria) {
   return function(scope, elem, attr) {
-    if ($aria.config('tabindex') && !elem.attr('tabindex')) {
+    if ($aria.config('tabindex') && !elem.attr('tabindex') && !isNodeOneOf(elem, nodeBlackList)) {
       elem.attr('tabindex', 0);
     }
   };
diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js
index 864f856ed271..911e6aab73d5 100644
--- a/test/ngAria/ariaSpec.js
+++ b/test/ngAria/ariaSpec.js
@@ -616,10 +616,35 @@ describe('$aria', function() {
   describe('tabindex', function() {
     beforeEach(injectScopeAndCompiler);
 
-    it('should attach tabindex to role="checkbox", ng-click, and ng-dblclick', function() {
+    it('should not attach to native controls', function() {
+      var element = [
+        $compile("<button ng-click='something'></button>")(scope),
+        $compile("<a ng-href='#/something'>")(scope),
+        $compile("<input ng-model='val'>")(scope),
+        $compile("<textarea ng-model='val'></textarea>")(scope),
+        $compile("<select ng-model='val'></select>")(scope),
+        $compile("<details ng-model='val'></details>")(scope)
+      ];
+      expectAriaAttrOnEachElement(element, 'tabindex', undefined);
+    });
+
+    it('should not attach to random ng-model elements', function() {
+      compileElement('<div ng-model="val"></div>');
+      expect(element.attr('tabindex')).toBeUndefined();
+    });
+
+    it('should attach tabindex to custom inputs', function() {
+      compileElement('<div type="checkbox" ng-model="val"></div>');
+      expect(element.attr('tabindex')).toBe('0');
+
       compileElement('<div role="checkbox" ng-model="val"></div>');
       expect(element.attr('tabindex')).toBe('0');
 
+      compileElement('<div type="range" ng-model="val"></div>');
+      expect(element.attr('tabindex')).toBe('0');
+    });
+
+    it('should attach to ng-click and ng-dblclick', function() {
       compileElement('<div ng-click="someAction()"></div>');
       expect(element.attr('tabindex')).toBe('0');
 
@@ -640,26 +665,6 @@ describe('$aria', function() {
       compileElement('<div ng-dblclick="someAction()" tabindex="userSetValue"></div>');
       expect(element.attr('tabindex')).toBe('userSetValue');
     });
-
-    it('should set proper tabindex values for radiogroup', function() {
-      compileElement('<div role="radiogroup">' +
-                     '<div role="radio" ng-model="val" value="one">1</div>' +
-                     '<div role="radio" ng-model="val" value="two">2</div>' +
-                   '</div>');
-
-      var one = element.contents().eq(0);
-      var two = element.contents().eq(1);
-
-      scope.$apply("val = 'one'");
-      expect(one.attr('tabindex')).toBe('0');
-      expect(two.attr('tabindex')).toBe('-1');
-
-      scope.$apply("val = 'two'");
-      expect(one.attr('tabindex')).toBe('-1');
-      expect(two.attr('tabindex')).toBe('0');
-
-      dealoc(element);
-    });
   });
 
   describe('accessible actions', function() {