Add an accessibility mode for editable text fields only.

Add an accessibility mode for editable text fields only.

This refactors the triggering of accessibility mode from a boolean state and
command-line flag to an AccessibilityMode enum that's passed directly
when a new RenderView is created, which is also more readable and avoids the
extra command-line flag.

Adds a new mode that only syncs editable text nodes, and enables this on
Windows 8 so that we can support showing the virtual keyboard when an
editable text field gets focus.

BUG=122061, 118641
TEST=Adds new test


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132267

[David Tseng, 2012-04-05 01:01:24]

http://codereview.chromium.org/9939011/diff/1/content/browser/accessibility/r...
File content/browser/accessibility/renderer_accessibility_browsertest.cc
(right):

http://codereview.chromium.org/9939011/diff/1/content/browser/accessibility/r...
content/browser/accessibility/renderer_accessibility_browsertest.cc:479: " 
<li><div role=textbox>text5</div>"
Maybe mix in a non-text input leaf node here?

http://codereview.chromium.org/9939011/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/render_view_host_impl.cc (right):

http://codereview.chromium.org/9939011/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/render_view_host_impl.cc:250:
AccessibilityModeComplete :
nit: indent looks wrong here.

http://codereview.chromium.org/9939011/diff/1/content/renderer/renderer_acces...
File content/renderer/renderer_accessibility.cc (right):

http://codereview.chromium.org/9939011/diff/1/content/renderer/renderer_acces...
content/renderer/renderer_accessibility.cc:611: // In Editable Text mode, we
send a "collapsed" tree of only editable
Ignoring notifications seems risky; what happens if a children changed
notification gets fired on a parent of a text field that then removes the text
field?

[dmazzoni, 2012-04-06 21:47:22]

PTAL; addressed review comments and I think I have it building on all platforms
now.

This does most of the refactoring. We'll need to optimize a bit more if this
ends up working out, but that should require touching less code.

http://codereview.chromium.org/9939011/diff/1/content/browser/accessibility/r...
File content/browser/accessibility/renderer_accessibility_browsertest.cc
(right):

http://codereview.chromium.org/9939011/diff/1/content/browser/accessibility/r...
content/browser/accessibility/renderer_accessibility_browsertest.cc:479: " 
<li><div role=textbox>text5</div>"
On 2012/04/05 01:01:25, David Tseng wrote:
> Maybe mix in a non-text input leaf node here?

Done.

http://codereview.chromium.org/9939011/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/render_view_host_impl.cc (right):

http://codereview.chromium.org/9939011/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/render_view_host_impl.cc:250:
AccessibilityModeComplete :
On 2012/04/05 01:01:25, David Tseng wrote:
> nit: indent looks wrong here.

Done.

http://codereview.chromium.org/9939011/diff/1/content/renderer/renderer_acces...
File content/renderer/renderer_accessibility.cc (right):

http://codereview.chromium.org/9939011/diff/1/content/renderer/renderer_acces...
content/renderer/renderer_accessibility.cc:611: // In Editable Text mode, we
send a "collapsed" tree of only editable
On 2012/04/05 01:01:25, David Tseng wrote:
> Ignoring notifications seems risky; what happens if a children changed
> notification gets fired on a parent of a text field that then removes the text
> field?

Actually it turns out I can get rid of this code path because it will never be
called. If a notification is sent on a node other than an editable text node,
the logic in SendPendingAccessibilityNotifications will go up to the root (see
the comment starting with "The browser may not have this object yet...").

I modified this method to return void.

We should add more optimizations later to send as few notifications as possible,
but this seems to work now.

[David Tseng, 2012-04-09 17:57:33]

lgtm

http://codereview.chromium.org/9939011/diff/12032/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

http://codereview.chromium.org/9939011/diff/12032/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:658: void
RenderWidgetHostImpl::EnableFullAccessibilityMode() {
It looks like this is only referenced once; do we need this?

[dmazzoni, 2012-04-09 18:51:47]

http://codereview.chromium.org/9939011/diff/12032/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

http://codereview.chromium.org/9939011/diff/12032/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:658: void
RenderWidgetHostImpl::EnableFullAccessibilityMode() {
On 2012/04/09 17:57:34, David Tseng wrote:
> It looks like this is only referenced once; do we need this?

The reason for this is because the AccessibilityMode enum is only part of the
content impl, we can't make it part of the public interface. So adding this
method seemed like the easiest solution.

[dmazzoni, 2012-04-09 18:56:14]

tony: approval for webkit/
jam: please review everything when you have a chance, most of the change is
refactoring how accessibility in the renderer process is enabled.

[ananta, 2012-04-12 22:55:14]

LGTM

[jam, 2012-04-12 23:12:05]

lgtm

sorry for the delay, I just saw this. in the future please feel free to IM me if
I don't respond in a few hours

[dmazzoni, 2012-04-12 23:24:05]

Oops, forgot Tony is out - Darin, can you approve the tiny change in webkit/?

[darin (slow to review), 2012-04-13 19:46:26]

LGTM for webkit/

[commit-bot: I haz the power, 2012-04-13 19:50:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/9939011/12032

[commit-bot: I haz the power, 2012-04-13 21:16:31]

Change committed as 132267