AX: VoiceOver appears unresponsive when JavaScript alerts are triggered via focus...
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Feb 2015 07:09:50 +0000 (07:09 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Feb 2015 07:09:50 +0000 (07:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=140485

Reviewed by Anders Carlsson.

Source/WebCore:

If setting an accessibility attribute results in a modal alert being displayed, it can cause VoiceOver
to hang. A simple solution is perform the actual work after a short delay, which will ensure the call
returns without hanging.

Test: platform/mac/accessibility/setting-attributes-is-asynchronous.html

* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper accessibilitySetValue:forAttribute:]):
(-[WebAccessibilityObjectWrapper _accessibilitySetValue:forAttribute:]):

Tools:

Implement takeFocus() as a way to set focus through accessibility wrappers.

* DumpRenderTree/mac/AccessibilityUIElementMac.mm:
(AccessibilityUIElement::takeFocus):
* WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::takeFocus):

LayoutTests:

Modify tests that relied on setting behavior and immediately checking results. Those
tests now need to retrieve results after a short timeout.

* accessibility/textarea-selected-text-range-expected.txt:
* accessibility/textarea-selected-text-range.html:
* platform/mac/accessibility/select-element-selection-with-optgroups.html:
* platform/mac/accessibility/setting-attributes-is-asynchronous-expected.txt: Added.
* platform/mac/accessibility/setting-attributes-is-asynchronous.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179820 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/accessibility/textarea-selected-text-range-expected.txt
LayoutTests/accessibility/textarea-selected-text-range.html
LayoutTests/platform/mac/accessibility/select-element-selection-with-optgroups.html
LayoutTests/platform/mac/accessibility/setting-attributes-is-asynchronous-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/accessibility/setting-attributes-is-asynchronous.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
Tools/ChangeLog
Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm
Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm

index 127913e..4371653 100644 (file)
@@ -1,3 +1,19 @@
+2015-02-08  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: VoiceOver appears unresponsive when JavaScript alerts are triggered via focus or blur events
+        https://bugs.webkit.org/show_bug.cgi?id=140485
+
+        Reviewed by Anders Carlsson.
+
+        Modify tests that relied on setting behavior and immediately checking results. Those
+        tests now need to retrieve results after a short timeout.
+
+        * accessibility/textarea-selected-text-range-expected.txt:
+        * accessibility/textarea-selected-text-range.html:
+        * platform/mac/accessibility/select-element-selection-with-optgroups.html:
+        * platform/mac/accessibility/setting-attributes-is-asynchronous-expected.txt: Added.
+        * platform/mac/accessibility/setting-attributes-is-asynchronous.html: Added.
+
 2015-02-08  Benjamin Poulain  <benjamin@webkit.org>
 
         Add parsing support for CSS Selector L4's case-insensitive attribute
index 130885b..3ef9b4b 100644 (file)
@@ -1,7 +1,7 @@
-(4,0) = {4, 0}
-
-(8,2) = {8, 2}
-
-(100,0) = {25, 0}
+PASS textArea.selectedTextRange is '{4, 0}'
+PASS textArea.selectedTextRange is '{8, 2}'
+PASS textArea.selectedTextRange is '{25, 0}'
+PASS successfullyParsed is true
 
+TEST COMPLETE
 
index 9fe7fe4..1e636d7 100644 (file)
@@ -1,8 +1,5 @@
 <html>
-<script>
-    if (window.testRunner)
-        testRunner.dumpAsText();
-</script>
+<script src="../resources/js-test-pre.js"></script>
 <body>
     
     <div id="result"></div>
@@ -15,23 +12,31 @@ line 3
 
     <script>
         if (window.accessibilityController) {
-            var result = document.getElementById("result");
-
+            window.jsTestIsAsync = true;
             var area1 = document.getElementById("area1");
             area1.focus();
 
             var textArea = accessibilityController.focusedElement;
 
             textArea.setSelectedTextRange(4,0);
-            result.innerText += "(4,0) = " + textArea.selectedTextRange + "\n\n";
-
-            textArea.setSelectedTextRange(8,2);
-            result.innerText += "(8,2) = " + textArea.selectedTextRange + "\n\n";
 
-            textArea.setSelectedTextRange(100,0);
-            result.innerText += "(100,0) = " + textArea.selectedTextRange + "\n\n";
+            // After setting a property through accessibility, the value won't be updated immediately, so we
+            // must check after a timeout to re-verify the value.
+            setTimeout(function() {
+                shouldBe("textArea.selectedTextRange", "'{4, 0}'");
+                textArea.setSelectedTextRange(8,2);
+                setTimeout(function() {
+                    shouldBe("textArea.selectedTextRange", "'{8, 2}'");
+                    textArea.setSelectedTextRange(100,0);
+                    setTimeout(function() {
+                        shouldBe("textArea.selectedTextRange", "'{25, 0}'");
+                        finishJSTest();
+                    }, 1);
+                }, 1);
+            }, 1);
 
         }
     </script>
+<script src="../resources/js-test-post.js"></script>
 </body>
 </html>
index 2b66167..560fb94 100644 (file)
@@ -26,6 +26,7 @@
     description("This tests that setting selection within a list box works correctly if there are optgroups");
 
     if (window.accessibilityController) {
+          window.jsTestIsAsync = true;
 
           document.getElementById("suite").focus();
           var selectElement = accessibilityController.focusedElement;
           var option2 = selectElement.childAtIndex(2);
           var option3 = selectElement.childAtIndex(4);
 
+          // Selection operations happen after a delay so they don't hang. Check the result on a timeout.
           selectElement.setSelectedChild(option1);
-          shouldBe("selectElement.selectedChildrenCount", "1");
-          shouldBeTrue("selectElement.selectedChildAtIndex(0).isEqual(option1)");
+          setTimeout(function() {
+              shouldBe("selectElement.selectedChildrenCount", "1");
+              shouldBeTrue("selectElement.selectedChildAtIndex(0).isEqual(option1)");
 
-          selectElement.setSelectedChild(option2);
-          shouldBe("selectElement.selectedChildrenCount", "1");
-          shouldBeTrue("selectElement.selectedChildAtIndex(0).isEqual(option2)");
+              selectElement.setSelectedChild(option2);
+              setTimeout(function() {
+                  shouldBe("selectElement.selectedChildrenCount", "1");
+                  shouldBeTrue("selectElement.selectedChildAtIndex(0).isEqual(option2)");
 
-          selectElement.setSelectedChild(option3);
-          shouldBe("selectElement.selectedChildrenCount", "1");
-          shouldBeTrue("selectElement.selectedChildAtIndex(0).isEqual(option3)");
+                  selectElement.setSelectedChild(option3);
+                  setTimeout(function() {
+                      shouldBe("selectElement.selectedChildrenCount", "1");
+                      shouldBeTrue("selectElement.selectedChildAtIndex(0).isEqual(option3)");
+                      finishJSTest();
+                  }, 1);
+              }, 1);
+           }, 1);
+        
     }
 
 </script>
diff --git a/LayoutTests/platform/mac/accessibility/setting-attributes-is-asynchronous-expected.txt b/LayoutTests/platform/mac/accessibility/setting-attributes-is-asynchronous-expected.txt
new file mode 100644 (file)
index 0000000..fc5e86e
--- /dev/null
@@ -0,0 +1,11 @@
+
+This tests makes sure that setting accessibility attributes happens asychronously, so that a caller won't hang if the result is an alert.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+This line should be printed first
+This line should be printed second.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/platform/mac/accessibility/setting-attributes-is-asynchronous.html b/LayoutTests/platform/mac/accessibility/setting-attributes-is-asynchronous.html
new file mode 100644 (file)
index 0000000..f59aa10
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<script src="../../../resources/js-test-pre.js"></script>
+
+<input type="text" id="textfield" onfocus="debug('This line should be printed second.');  finishJSTest(); ">
+
+<div id="description"></div>
+<div id="console"></div>
+
+<script>
+
+description("This tests makes sure that setting accessibility attributes happens asychronously, so that a caller won't hang if the result is an alert.")
+
+if (window.testRunner && window.accessibilityController) {
+    
+   window.jsTestIsAsync = true;
+   var textfield = accessibilityController.accessibleElementById("textfield");
+   textfield.takeFocus();
+   debug("This line should be printed first");
+}
+
+</script>
+
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
index 1c5a258..e6dd1ec 100644 (file)
@@ -1,3 +1,20 @@
+2015-02-08  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: VoiceOver appears unresponsive when JavaScript alerts are triggered via focus or blur events
+        https://bugs.webkit.org/show_bug.cgi?id=140485
+
+        Reviewed by Anders Carlsson.
+
+        If setting an accessibility attribute results in a modal alert being displayed, it can cause VoiceOver
+        to hang. A simple solution is perform the actual work after a short delay, which will ensure the call
+        returns without hanging.
+
+        Test: platform/mac/accessibility/setting-attributes-is-asynchronous.html
+
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (-[WebAccessibilityObjectWrapper accessibilitySetValue:forAttribute:]):
+        (-[WebAccessibilityObjectWrapper _accessibilitySetValue:forAttribute:]):
+
 2015-02-08  Benjamin Poulain  <benjamin@webkit.org>
 
         Add parsing support for CSS Selector L4's case-insensitive attribute
index 84bdc2f..4bd7546 100644 (file)
@@ -3246,6 +3246,15 @@ static NSString* roleValueToNSString(AccessibilityRole value)
 
 - (void)accessibilitySetValue:(id)value forAttribute:(NSString*)attributeName
 {
+    // In case anything we do by changing values causes an alert or other modal
+    // behaviors, we need to return now, so that VoiceOver doesn't hang indefinitely.
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [self _accessibilitySetValue:value forAttribute:attributeName];
+    });
+}
+
+- (void)_accessibilitySetValue:(id)value forAttribute:(NSString*)attributeName
+{
     if (![self updateObjectBackingStore])
         return;
     
index dae1f93..3f0e074 100644 (file)
@@ -1,3 +1,17 @@
+2015-02-08  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: VoiceOver appears unresponsive when JavaScript alerts are triggered via focus or blur events
+        https://bugs.webkit.org/show_bug.cgi?id=140485
+
+        Reviewed by Anders Carlsson.
+
+        Implement takeFocus() as a way to set focus through accessibility wrappers.
+
+        * DumpRenderTree/mac/AccessibilityUIElementMac.mm:
+        (AccessibilityUIElement::takeFocus):
+        * WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
+        (WTR::AccessibilityUIElement::takeFocus):
+
 2015-02-08  Darin Adler  <darin@apple.com>
 
         Remove the SVG instance tree
index 26a76e0..e73abe8 100644 (file)
@@ -1517,7 +1517,9 @@ bool AccessibilityUIElement::hasPopup() const
 
 void AccessibilityUIElement::takeFocus()
 {
-    // FIXME: implement
+    BEGIN_AX_OBJC_EXCEPTIONS
+    [m_element accessibilitySetValue:@YES forAttribute:NSAccessibilityFocusedAttribute];
+    END_AX_OBJC_EXCEPTIONS
 }
 
 void AccessibilityUIElement::takeSelection()
index b7b9651..d0bb356 100644 (file)
@@ -168,6 +168,7 @@ interface AccessibilityUIElement {
     readonly attribute AccessibilityUIElement verticalScrollbar;
 
     void scrollToMakeVisible();
+    void takeFocus();
 
     // Text markers.
     AccessibilityTextMarkerRange lineTextMarkerRangeForTextMarker(AccessibilityTextMarker textMarker);
index 21cd677..fd52377 100644 (file)
@@ -1532,7 +1532,9 @@ bool AccessibilityUIElement::hasPopup() const
 
 void AccessibilityUIElement::takeFocus()
 {
-    // FIXME: implement
+    BEGIN_AX_OBJC_EXCEPTIONS
+    [m_element accessibilitySetValue:@YES forAttribute:NSAccessibilityFocusedAttribute];
+    END_AX_OBJC_EXCEPTIONS
 }
 
 void AccessibilityUIElement::takeSelection()