Bug 43005 – AX: Support methods for web apps to interact with the native accessibility APIs

• Component: Accessibility
• Status: RESOLVED WONTFIX

[chris fleizach]

With web apps becoming a popular option that can stand side-by-side with native apps, the need exists for web apps to be able to integrate with existing accessibility API more easily.

With that in mind, this patch extends Navigator to return an accessibility object. Within that is a screenReader object which can be queried.

Here is the initial API. Myself and members of the WAI-ARIA working group are working on a standard for this as well.


window.navigator.accessibility
… new accessibility object added as a property of window.navigator.

window.navigator.accessibility.screenChanged() 
… tells API entire screen has changed, update entire accessibility tree

window.navigator.accessibility.elementsChanged()
… tells API elements have changed and are finished changing for new (e.g. animation finished), assistive tech update any its interested in (like currently focused element)

window.navigator.accessibility.screenreader
… properties and methods specific to a screen reader. 

window.navigator.accessibility.screenreader.active
… true if the screen reader is running, false if not.

window.navigator.accessibility.screenreader.version
… string value similar to user agent string (e.g. 'Apple VoiceOver 3.0.1 (210.11)' )

window.navigator.accessibility. screenreader.activeElement
… VO cursor equivalent of document.activeElement for native kb focus

[chris fleizach]

this initial patch will fail on a lot of systems i imagine. will gather build output to fix

[chris fleizach]

Created attachment 62612 [details]
Patch

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> Created an attachment (id=62612) [details]
> Patch

Are you waiting for build output for this, or is the patch ready for review?

It looks like the bots all failed to apply the patch.

[David Kilzer (:ddkilzer)]

Comment on attachment 62612 [details]
Patch

r- to post another patch for the bots.

[chris fleizach]

(In reply to comment #3)
> (In reply to comment #2)
> > Created an attachment (id=62612) [details] [details]
> > Patch
> 
> Are you waiting for build output for this, or is the patch ready for review?
> 
> It looks like the bots all failed to apply the patch.

patch is ready, but i suspect it won't work on every build. will try again

[chris fleizach]

Created attachment 63068 [details]
Patch

[WebKit Review Bot]

Attachment 63068 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/page/Navigator.h:30:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/accessibility/AccessibilityAllInOne.cpp:29:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
Total errors found: 3 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

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

[chris fleizach]

Created attachment 63074 [details]
Patch

[WebKit Review Bot]

Attachment 63074 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/page/Navigator.h:30:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/accessibility/AccessibilityAllInOne.cpp:29:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
Total errors found: 3 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

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

[chris fleizach]

Created attachment 63082 [details]
Patch

[WebKit Review Bot]

Attachment 63082 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/DerivedSources.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/page/Navigator.h:30:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

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

[Early Warning System Bot]

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

[chris fleizach]

Created attachment 63086 [details]
Patch

[WebKit Review Bot]

Attachment 63086 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/DerivedSources.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/page/Navigator.h:30:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

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

[WebKit Review Bot]

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

[WebKit Review Bot]

Attachment 63086 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3573632

[chris fleizach]

Created attachment 63101 [details]
Patch

[Early Warning System Bot]

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

[Eric Seidel (no email)]

Attachment 63101 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3561639

[chris fleizach]

Created attachment 63102 [details]
Patch

[Early Warning System Bot]

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

[WebKit Review Bot]

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

[WebKit Review Bot]

Attachment 63101 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3649049

[WebKit Review Bot]

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

[chris fleizach]

Created attachment 63107 [details]
Patch

[WebKit Review Bot]

Attachment 63102 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3577665

[WebKit Review Bot]

Attachment 63107 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3640116

[chris fleizach]

Created attachment 63129 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 63129 [details]
Patch

WebCore/accessibility/Accessibility.cpp:13
 +   * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of

This third clause is no longer in the current license (see WebCore/LICENSE-APPLE).  Please update the license on all of the new files.

r=me

[chris fleizach]

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

[WebKit Review Bot]

http://trac.webkit.org/changeset/64471 might have broken Qt Linux Release

[chris fleizach]

(In reply to comment #35)
> http://trac.webkit.org/changeset/64471 might have broken Qt Linux Release

didn't break it but changed some layout test results. will update

[chris fleizach]

(In reply to comment #36)
> (In reply to comment #35)
> > http://trac.webkit.org/changeset/64471 might have broken Qt Linux Release
> 
> didn't break it but changed some layout test results. will update

updated tests that would be changed by this change

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

[chris fleizach]

(In reply to comment #37)
> (In reply to comment #36)
> > (In reply to comment #35)
> > > http://trac.webkit.org/changeset/64471 might have broken Qt Linux Release
> > 
> > didn't break it but changed some layout test results. will update
> 
> updated tests that would be changed by this change
> 
> http://trac.webkit.org/changeset/64478

QT is behaving wierdly. It's still failing

http://build.webkit.org/results/Qt%20Linux%20Release/r64480%20(16816)/results.html

but by all accounts it looks like it should have been fixed by

http://trac.webkit.org/changeset/64478. it would seem that that file was not updated?

[Csaba Osztrogonác]

Qt has its own fast/dom/Window/window-properties-expected.txt expected file,
and was updated by http://trac.webkit.org/changeset/64486

[Sam Weinig]

Hi Chris,

Without a spec or conversation with the rest of the WebKit community, I don't think we should be landing this in the tree, especially without any #define guards around it.  Please revert this until there has been the appropriate discussion.

[David Kilzer (:ddkilzer)]

(In reply to comment #40)
> Hi Chris,
> 
> Without a spec or conversation with the rest of the WebKit community, I don't think we should be landing this in the tree, especially without any #define guards around it.  Please revert this until there has been the appropriate discussion.

Sorry, I should have not given this an r+ without more discussion about the API.

It would be nice to have a basic outline for the WebKit API review process posted on webkit.org for future reference, even if it's just a list of mailing lists where you should send proposals for each standards body.

[chris fleizach]

Reverted r64486 for reason:

Rolling out https://bugs.webkit.org/show_bug.cgi?id=43005 until further discussion

Committed r64511: <http://trac.webkit.org/changeset/64511>

[chris fleizach]

Reverted r64478 for reason:

Rolling out https://bugs.webkit.org/show_bug.cgi?id=43005 until further discussion

Committed r64512: <http://trac.webkit.org/changeset/64512>

[WebKit Review Bot]

http://trac.webkit.org/changeset/64511 might have broken Qt Linux Release

[chris fleizach]

(In reply to comment #44)
> http://trac.webkit.org/changeset/64511 might have broken Qt Linux Release

I'm rolling out all changes associated with this patch right now

[chris fleizach]

Reverted r64476 for reason:

Rolling out https://bugs.webkit.org/show_bug.cgi?id=43005 until further discussion

Committed r64514: <http://trac.webkit.org/changeset/64514>

[chris fleizach]

Reverted r64472 for reason:

Rolling out https://bugs.webkit.org/show_bug.cgi?id=43005 until further discussion

Committed r64515: <http://trac.webkit.org/changeset/64515>

[chris fleizach]

Reverted r64471 for reason:

Rolling out https://bugs.webkit.org/show_bug.cgi?id=43005 until further discussion

Committed r64517: <http://trac.webkit.org/changeset/64517>

[Sam Weinig]

Thanks Chris.

[WebKit Review Bot]

http://trac.webkit.org/changeset/64517 might have broken Leopard Intel Debug (Tests)
The following changes are on the blame list:
http://trac.webkit.org/changeset/64517
http://trac.webkit.org/changeset/64518
http://trac.webkit.org/changeset/64519
http://trac.webkit.org/changeset/64520
http://trac.webkit.org/changeset/64521
http://trac.webkit.org/changeset/64522
http://trac.webkit.org/changeset/64523
http://trac.webkit.org/changeset/64524
http://trac.webkit.org/changeset/64525
http://trac.webkit.org/changeset/64526
http://trac.webkit.org/changeset/64527

[Adam Barth]

This bug is mysterious to me.  Does this patch need to be landed or do we want more review here?

[David Kilzer (:ddkilzer)]

Comment on attachment 63129 [details]
Patch

Clearing r+ flag.

[chris fleizach]

Comment on attachment 63129 [details]
Patch

going to hold off on this until we make a formal proposal

[chris fleizach]

we have made the proposal public for a number of weeks now. no major criticisms

http://lists.w3.org/Archives/Public/wai-xtech/2010Aug/att-0079/UserInterfaceIndependence.html

[chris fleizach]

Created attachment 67731 [details]
patch

This is the same patch that david kilzer reviewed before but also includes all the fixes i had made to get it to build on each platform.

[Eric Seidel (no email)]

Attachment 67731 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4014022

[chris fleizach]

Created attachment 67736 [details]
patch

[Adam Barth]

Comment on attachment 67736 [details]
patch

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

Seems reasonable.  Some nits below.

> WebCore/accessibility/AXObjectCache.h:47
> +#if PLATFORM(MAC)
> +#define SUPPORTS_JSACCESSIBILITY 1
> +#else
> +#define SUPPORTS_JSACCESSIBILITY 0
> +#endif

This isn't the way we usually enable features.  We usually use ENABLE macros?

> WebCore/accessibility/Accessibility.cpp:60
> +    Document* doc = m_frame->document();

Please use document as the variable name for the document.

> WebCore/accessibility/Accessibility.cpp:62
> +    if (!doc)
> +        return;

m_frame->document() can never be null.

> WebCore/accessibility/Accessibility.cpp:64
> +    AccessibilityObject* root = doc->axObjectCache()->getOrCreate(doc->renderer());

Is there any chance of this code running JavaScript?  If so, we need to hold a reference to the document.

> WebCore/accessibility/Accessibility.cpp:75
> +    if (!doc)
> +        return;

m_frame->document() can never be null.

> WebCore/accessibility/Accessibility.idl:33
> +

This line sounds extra.

> WebCore/accessibility/ScreenReader.h:47
> +private:

missing a newline above here.

[James Craig]

Given that this section never made it into IndieUI, should we close this one as WONTFIX?

[chris fleizach]

(In reply to comment #59)
> Given that this section never made it into IndieUI, should we close this one as WONTFIX?

Yes

[James Craig]

Closing