Bug 51859 – WK2: Support Accessibility

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

Need to support accessibility for the Mac

[chris fleizach]

Created attachment 77896 [details]
Patch

[WebKit Review Bot]

Attachment 77896 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7314339

[Early Warning System Bot]

Attachment 77896 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7246433

[chris fleizach]

Created attachment 77907 [details]
Patch

[Build Bot]

Attachment 77896 [details] did not build on win:
Build output: http://queues.webkit.org/results/7338296

[Early Warning System Bot]

Attachment 77907 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7327339

[chris fleizach]

Created attachment 77914 [details]
Patch

[Build Bot]

Attachment 77907 [details] did not build on win:
Build output: http://queues.webkit.org/results/7243408

[Early Warning System Bot]

Attachment 77914 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7201407

[Build Bot]

Attachment 77914 [details] did not build on win:
Build output: http://queues.webkit.org/results/7235431

[chris fleizach]

Created attachment 77921 [details]
Patch

[Build Bot]

Attachment 77921 [details] did not build on win:
Build output: http://queues.webkit.org/results/7288367

[chris fleizach]

Created attachment 77929 [details]
Patch

[Sam Weinig]

Comment on attachment 77929 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77929&action=review

This overall looks really good. Maybe Anders can give it a once over as well.

> Tools/DumpRenderTree/mac/AccessibilityControllerMac.mm:31
> +#import <AppKit/NSColor.h>

What is this for?  It doesn't seem related.

> WebCore/accessibility/AccessibilityScrollView.cpp:127
> +}
> +
> +    
> +AccessibilityObject* AccessibilityScrollView::accessibilityHitTest(const IntPoint& point) const

Extra space.

> WebCore/accessibility/AccessibilityScrollbar.cpp:39
> +AccessibilityScrollbar::AccessibilityScrollbar(Scrollbar *scrollbar)

* on the wrong side.

> WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:1027
>  
> +    else if (m_object->isScrollView())
> +        objectAttributes = scrollViewAttrs;

The newline seems unnecessary.

> WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:1125
> +    }
> +    else {

The } should be on the same line as the else.

> WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:1131
> +            point = [[view window] convertBaseToScreen: [view convertPoint: point toView:nil]];

We usually don't put a space after the :.

> WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:1459
> +    if (scroll->platformWidget())
> +        return scroll->platformWidget();
> +    else
> +        return [self remoteAccessibilityParentObject];

The else is not necessary here.

> WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:1730
>          }
> +        else if (m_object->isScrollView()) {
> +            AccessibilityObject::AccessibilityChildrenVector children = m_object->children();

} should be on the same line as the else if.

[chris fleizach]

(In reply to comment #14)
> (From update of attachment 77929 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77929&action=review
> 
> This overall looks really good. Maybe Anders can give it a once over as well.
> 
> > Tools/DumpRenderTree/mac/AccessibilityControllerMac.mm:31
> > +#import <AppKit/NSColor.h>
> 
> What is this for?  It doesn't seem related.
> 

AccessibilityControllerMac.mm now includes WebFramePrivate instead of Internal. WebFramePrivate has a NSColor* in its header.

suggestions for what is expected here are welcome

will do everything else. thanks

[chris fleizach]

http://trac.webkit.org/changeset/75031

[WebKit Review Bot]

http://trac.webkit.org/changeset/75031 might have broken SnowLeopard Intel Release (Build)

[chris fleizach]

http://trac.webkit.org/changeset/75033

[chris fleizach]

http://trac.webkit.org/changeset/75034

[chris fleizach]

I created https://bugs.webkit.org/show_bug.cgi?id=51911 to handle the GTK layout test failure and other GTK related issues

[chris fleizach]

I made https://bugs.webkit.org/show_bug.cgi?id=51912 to handle the failing Windows tests

[WebKit Review Bot]

http://trac.webkit.org/changeset/75033 might have broken Leopard Intel Debug (Tests)

[WebKit Review Bot]

http://trac.webkit.org/changeset/75034 might have broken Leopard Intel Debug (Tests)

[WebKit Review Bot]

http://trac.webkit.org/changeset/75052 might have broken Leopard Intel Debug (Tests)

[Ryosuke Niwa]

http://trac.webkit.org/changeset/75031 seems to broke WIn7 accessibility tests:
http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r75022%20(7918)/
http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r75031%20(7919)/

--- /home/buildbot/slave/WebKit-BuildSlave/win-release-tests/build/layout-test-results/platform/win/accessibility/document-role-expected.txt	2011-01-05 00:33:06.263629600 -0800
+++ /home/buildbot/slave/WebKit-BuildSlave/win-release-tests/build/layout-test-results/platform/win/accessibility/document-role-actual.txt	2011-01-05 00:33:06.261629400 -0800
@@ -1,4 +1,4 @@
 Test for role attribute of document 
 ----------------------------------- 
-role of Document : document 
+role of Document : client 

--- /home/buildbot/slave/WebKit-BuildSlave/win-release-tests/build/layout-test-results/platform/win/accessibility/selection-and-focus-expected.txt	2011-01-05 00:33:07.034673700 -0800
+++ /home/buildbot/slave/WebKit-BuildSlave/win-release-tests/build/layout-test-results/platform/win/accessibility/selection-and-focus-actual.txt	2011-01-05 00:33:07.032673500 -0800
@@ -2,7 +2,7 @@
 This tests that we can adjust focus and selection via accessibility API.
 
 
-PASS accessibilityController.focusedElement.role is "document"
+FAIL accessibilityController.focusedElement.role should be document. Was client.
 PASS accessibilityController.focusedElement.role is "list"
 
 Option 0 is selected.

[Ryosuke Niwa]

Apparently, the regression is tracked by https://bugs.webkit.org/show_bug.cgi?id=51912.