| Summary: | WK2: Support Accessibility | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||||||||||
| Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | abarth, andersca, buildbot, dglazkov, eric, rniwa, sam, webkit-ews, webkit.review.bot | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Mac | ||||||||||||||
| OS: | OS X 10.5 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
chris fleizach
2011-01-03 18:01:58 PST
Created attachment 77896 [details]
Patch
Attachment 77896 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7314339 Attachment 77896 [details] did not build on qt: Build output: http://queues.webkit.org/results/7246433 Created attachment 77907 [details]
Patch
Attachment 77896 [details] did not build on win: Build output: http://queues.webkit.org/results/7338296 Attachment 77907 [details] did not build on qt: Build output: http://queues.webkit.org/results/7327339 Created attachment 77914 [details]
Patch
Attachment 77907 [details] did not build on win: Build output: http://queues.webkit.org/results/7243408 Attachment 77914 [details] did not build on qt: Build output: http://queues.webkit.org/results/7201407 Attachment 77914 [details] did not build on win: Build output: http://queues.webkit.org/results/7235431 Created attachment 77921 [details]
Patch
Attachment 77921 [details] did not build on win: Build output: http://queues.webkit.org/results/7288367 Created attachment 77929 [details]
Patch
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. (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 http://trac.webkit.org/changeset/75031 might have broken SnowLeopard Intel Release (Build) I created https://bugs.webkit.org/show_bug.cgi?id=51911 to handle the GTK layout test failure and other GTK related issues I made https://bugs.webkit.org/show_bug.cgi?id=51912 to handle the failing Windows tests http://trac.webkit.org/changeset/75033 might have broken Leopard Intel Debug (Tests) http://trac.webkit.org/changeset/75034 might have broken Leopard Intel Debug (Tests) http://trac.webkit.org/changeset/75052 might have broken Leopard Intel Debug (Tests) 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. Apparently, the regression is tracked by https://bugs.webkit.org/show_bug.cgi?id=51912. |