diff --git a/PRESUBMIT.py b/PRESUBMIT.py index ef565c79b12d34..1f782832109217 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -5873,6 +5873,52 @@ def FileFilter(affected_file): return [output_api.PresubmitNotifyResult(message)] + +_ACCESSIBILITY_ARIA_METHOD_CANDIDATES_PATTERNS = r'(\-\>|\.)(get|has|FastGet|FastHas)Attribute\(' + +_ACCESSIBILITY_ARIA_BAD_PARAMS_PATTERNS = ( + r"\(html_names::kAria(.*)Attr\)", + r"\(html_names::kRoleAttr\)" +) + +_ACCESSIBILITY_ARIA_FILE_CANDIDATES_PATTERNS = ( + r".*/accessibility/.*.(cc|h)", + r".*/ax_.*.(cc|h)" +) + +def CheckAccessibilityAriaElementAttributeGetters(input_api, output_api): + """Checks that the blink accessibility code follows the defined patterns + for checking aria attributes, so that ElementInternals is not bypassed.""" + + # Limit to accessibility-related files. + def FileFilter(affected_file): + paths = _ACCESSIBILITY_ARIA_FILE_CANDIDATES_PATTERNS + return input_api.FilterSourceFile(affected_file, files_to_check=paths) + + aria_method_regex = input_api.re.compile(_ACCESSIBILITY_ARIA_METHOD_CANDIDATES_PATTERNS) + aria_bad_params_regex = input_api.re.compile( + "|".join(_ACCESSIBILITY_ARIA_BAD_PARAMS_PATTERNS) + ) + problems = [] + + for f in input_api.AffectedSourceFiles(FileFilter): + for line_num, line in f.ChangedContents(): + if aria_method_regex.search(line) and aria_bad_params_regex.search(line): + problems.append(f"{f.LocalPath()}:{line_num}\n {line.strip()}") + + if problems: + return [ + output_api.PresubmitPromptWarning( + "Accessibility code should not use element methods to get or check" + "\nthe presence of aria attributes" + "\nPlease use ARIA-specific attribute access, e.g. HasAriaAttribute()," + "\nAriaTokenAttribute(), AriaBoolAttribute(), AriaBooleanAttribute()," + "\nAriaFloatAttribute().", + problems, + ) + ] + return [] + # string pattern, sequence of strings to show when pattern matches, # error flag. True if match is a presubmit error, otherwise it's a warning. _NON_INCLUSIVE_TERMS = ( diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index c1a628ee2f7935..8a1bdff3757c89 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -1289,6 +1289,95 @@ def testRelnotesLowercaseWithEqualSign(self): 'Expected %d messages, found %d: %s' % (0, len(msgs), msgs)) +class AccessibilityAriaElementAttributeGettersTest(unittest.TestCase): + + # Test warning is surfaced for various possible uses of bad methods. + def testMatchingLines(self): + mock_input_api = MockInputApi() + mock_input_api.files = [ + MockFile( + "third_party/blink/renderer/core/accessibility/ax_object.h", + [ + "->getAttribute(html_names::kAriaCheckedAttr)", + "node->hasAttribute(html_names::kRoleAttr)", + "->FastHasAttribute(html_names::kAriaLabelAttr)", + " .FastGetAttribute(html_names::kAriaCurrentAttr);", + + ], + action='M' + ), + MockFile( + "third_party/blink/renderer/core/accessibility/ax_table.cc", + [ + "bool result = node->hasAttribute(html_names::kFooAttr);", + "foo->getAttribute(html_names::kAriaInvalidValueAttr)", + "foo->GetAriaCurrentState(html_names::kAriaCurrentStateAttr)", + ], + action='M' + ), + ] + + results = PRESUBMIT.CheckAccessibilityAriaElementAttributeGetters(mock_input_api, MockOutputApi()) + self.assertEqual(1, len(results)) + self.assertEqual(5, len(results[0].items)) + self.assertIn("ax_object.h:1", results[0].items[0]) + self.assertIn("ax_object.h:2", results[0].items[1]) + self.assertIn("ax_object.h:3", results[0].items[2]) + self.assertIn("ax_object.h:4", results[0].items[3]) + self.assertIn("ax_table.cc:2", results[0].items[4]) + self.assertIn("Please use ARIA-specific attribute access", results[0].message) + + # Test no warnings for files that are not accessibility related. + def testNonMatchingFiles(self): + mock_input_api = MockInputApi() + mock_input_api.files = [ + MockFile( + "content/browser/foobar/foo.cc", + ["->getAttribute(html_names::kAriaCheckedAttr)"], + action='M'), + MockFile( + "third_party/blink/renderer/core/foo.cc", + ["node->hasAttribute(html_names::kRoleAttr)"], + action='M'), + ] + results = PRESUBMIT.CheckAccessibilityAriaElementAttributeGetters(mock_input_api, MockOutputApi()) + self.assertEqual(0, len(results)) + + # Test no warning when methods are used with different attribute params. + def testNoBadParam(self): + mock_input_api = MockInputApi() + mock_input_api.files = [ + MockFile( + "third_party/blink/renderer/core/accessibility/ax_object.h", + [ + "->getAttribute(html_names::kCheckedAttr)", + "->hasAttribute(html_names::kIdAttr)", + ], + action='M' + ) + ] + + results = PRESUBMIT.CheckAccessibilityAriaElementAttributeGetters(mock_input_api, MockOutputApi()) + self.assertEqual(0, len(results)) + + # Test no warning when attribute params are used for different methods. + def testNoMethod(self): + mock_input_api = MockInputApi() + mock_input_api.files = [ + MockFile( + "third_party/blink/renderer/core/accessibility/ax_object.cc", + [ + "foo(html_names::kAriaCheckedAttr)", + "bar(html_names::kRoleAttr)" + ], + action='M' + ) + ] + + results = PRESUBMIT.CheckAccessibilityAriaElementAttributeGetters(mock_input_api, MockOutputApi()) + self.assertEqual(0, len(results)) + + class AndroidDeprecatedTestAnnotationTest(unittest.TestCase): def testCheckAndroidTestAnnotationUsage(self):