Fix Android accessibility for editable text fields

Fix Android accessibility for editable text fields

For text fields we need to set the text using setText, not
setContentDescription, and we also need to call setEditable(true),
both on the AccessibilityNodeInfo for the editable text field.

See bug for details, this allows you to use cursor controls in
TalkBack.

BUG=592297
Committed: https://crrev.com/a21c5040b40f34ea13ce37c85f0f1036b16e124a
Cr-Commit-Position: refs/heads/master@{#387054}

[dmazzoni, 2016-04-08 18:49:58]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org

[David Tseng, 2016-04-11 16:48:35]

https://codereview.chromium.org/1872013002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager_android.cc
(right):

https://codereview.chromium.org/1872013002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_android.cc:406: if
(!node->IsPassword() ||
How do we currently work with input type password?

https://codereview.chromium.org/1872013002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/BrowserAccessibilityManager.java
(right):

https://codereview.chromium.org/1872013002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/accessibility/BrowserAccessibilityManager.java:867:
node.setText(charSequence);
What happens with a text field containing both a value and name?

[dmazzoni, 2016-04-11 20:49:06]

https://codereview.chromium.org/1872013002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager_android.cc
(right):

https://codereview.chromium.org/1872013002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_android.cc:406: if
(!node->IsPassword() ||
On 2016/04/11 16:48:34, David Tseng wrote:
> How do we currently work with input type password?

There's an Android setting that determines whether it's
echoed or not, that's what this checks. So if it is
echoed, we use this API to announce it whether it's a
password field or not. If it is not echoed we don't set
the text.

https://codereview.chromium.org/1872013002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/accessibility/BrowserAccessibilityManager.java
(right):

https://codereview.chromium.org/1872013002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/accessibility/BrowserAccessibilityManager.java:867:
node.setText(charSequence);
On 2016/04/11 16:48:34, David Tseng wrote:
> What happens with a text field containing both a value and name?

Right now only the value is used because Android doesn't have
a way to express both. There's a bug filed to add a new API for
that.

[David Tseng, 2016-04-11 21:01:42]

https://codereview.chromium.org/1872013002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager_android.cc
(right):

https://codereview.chromium.org/1872013002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_android.cc:406: if
(!node->IsPassword() ||
On 2016/04/11 20:49:06, dmazzoni wrote:
> On 2016/04/11 16:48:34, David Tseng wrote:
> > How do we currently work with input type password?
> 
> There's an Android setting that determines whether it's
> echoed or not, that's what this checks. So if it is
> echoed, we use this API to announce it whether it's a
> password field or not. If it is not echoed we don't set
> the text.

I thought it wasn't so much that it was echoed, but the characters were
*/dotted-ed out?

[dmazzoni, 2016-04-12 18:18:26]

You're right, I think it makes more sense for us to return dots when it's in the
mode to obscure the password field.

I fixed it and just confirmed that this now behaves as expected if I don't tell
TalkBack that it's a password
field. However, if I do call setPassword(true) on the
node and event, TalkBack seems to not read out the dot
characters. It behaves similarly with native text fields
so I don't think this is our bug.

Either way, I think this behavior is correct on our end
now.

[David Tseng, 2016-04-12 18:35:35]

lgtm

As discussed offline, circle back with TalkBack team to iron out final desired
behavior.

[chromium-reviews, 2016-04-12 18:36:42]

Doesn't Blink already return dots or whatever is the mask character for 
the value attribute of password fields?

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-04-12 23:27:18]

OK, the TalkBack team confirmed this is correct.

Nektarios, we can't rely on Blink doing it because
on Android the behavior changes dynamically when
you plug or unplug headphones.

[dmazzoni, 2016-04-12 23:27:24]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-04-12 23:27:41]

The CQ bit was unchecked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2016-04-12 23:27:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1872013002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1872013002/20001

[dmazzoni, 2016-04-13 17:58:03]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-04-13 17:58:04]

The patchset sent to the CQ was uploaded after l-g-t-m from dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/1872013002/#ps40001
(title: "Update test expectation")

[commit-bot: I haz the power, 2016-04-13 17:58:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1872013002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1872013002/40001

[commit-bot: I haz the power, 2016-04-13 18:55:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-13 18:56:31]

Description was changed from

==========
Fix Android accessibility for editable text fields

For text fields we need to set the text using setText, not
setContentDescription, and we also need to call setEditable(true),
both on the AccessibilityNodeInfo for the editable text field.

See bug for details, this allows you to use cursor controls in
TalkBack.

BUG=592297
==========

to

==========
Fix Android accessibility for editable text fields

For text fields we need to set the text using setText, not
setContentDescription, and we also need to call setEditable(true),
both on the AccessibilityNodeInfo for the editable text field.

See bug for details, this allows you to use cursor controls in
TalkBack.

BUG=592297

Committed: https://crrev.com/a21c5040b40f34ea13ce37c85f0f1036b16e124a
Cr-Commit-Position: refs/heads/master@{#387054}
==========

[commit-bot: I haz the power, 2016-04-13 18:56:35]

Patchset 3 (id:??) landed as
https://crrev.com/a21c5040b40f34ea13ce37c85f0f1036b16e124a
Cr-Commit-Position: refs/heads/master@{#387054}