1473998 - Use EditText class names on editable accessibility nodes

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Eitan Isaacson [:eeejay]]

We currently provide a role name, a label, and the contents of an input value in the 'text' field of the node. This makes it hard to properly support text editing.

TalkBack expects the following:

1. If the className is 'android.widget.EditText', TalkBack will utter "Edit Box", in addition a text editing submenu will be added to TalkBack's local menu when the element is focused.
2. TalkBack will use the node's 'text' field for the actual text value of the entry.
3. Talkback will use a hint inserted into the 'extras' property bag as the label for the entry.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Use EditText entry in accessible's classNames and use the hint for the label. r?yzen r?jchen

This patch does a couple of other things:
- Remove checks for API less than 16.
- Changed the scroll events to be followed by window content changed events. That is the correct way to update the bounds on an accessible, not send more a11y focus events.

Comment 2

[Chris Peterson [:cpeterson]]

Assigning to Eitan since he posted a patch.

I assume we will want to uplift this fix to GV 62 Beta for the initial Focus+GV release.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8990469 [details] [diff] [review]
Use EditText entry in accessible's classNames and use the hint for the label. r?yzen r?jchen

Review of attachment 8990469 [details] [diff] [review]:
-----------------------------------------------------------------

r+ for Java bits.

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
@@ +3,5 @@
>     http://creativecommons.org/publicdomain/zero/1.0/ */
>  
>  package org.mozilla.geckoview.test
>  
> +import android.os.Bundle

Move to before `import android.support.test.filters.MediumTest`

@@ +61,5 @@
>          }
>      }
>  
> +    private fun createURL(html: String): String {
> +        return "data:text/html;base64," + Base64.getEncoder().encodeToString(html.toByteArray(Charset.forName("utf-8")))

Use `GeckoSession.createDataUri`

@@ +163,5 @@
> +            @AssertCalled(count = 1)
> +            override fun onAccessibilityFocused(event: AccessibilityEvent) {
> +                val nodeId = getSourceId(event)
> +                val node = provider.createAccessibilityNodeInfo(nodeId)
> +                android.util.Log.i("eeejay", node.toString())

Remove

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Use EditText entry in accessible's classNames and use the hint for the label. r?yzen r?jchen

Oh cool, session has a loadString method!

Comment 5

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8990800 [details] [diff] [review]
Use EditText entry in accessible's classNames and use the hint for the label. r?yzen r?jchen

Review of attachment 8990800 [details] [diff] [review]:
-----------------------------------------------------------------

Tanks, just a couple of nits.

::: accessible/jsat/Presentation.jsm
@@ +46,5 @@
>      }
>  
>      let androidEvents = [];
>  
> +    let isExploreByTouch = (aReason == Ci.nsIAccessiblePivot.REASON_POINT);

nit: let -> const and no reason for ()

@@ +55,5 @@
>        androidEvents.push({eventType: AndroidEvents.VIEW_HOVER_EXIT, text: []});
>      }
>  
>      if (aReason === Ci.nsIAccessiblePivot.REASON_TEXT) {
> +      let adjustedText = context.textAndAdjustedOffsets;

nit: const

@@ +143,5 @@
>     */
>    textSelectionChanged(aText, aStart, aEnd, aOldStart, aOldEnd, aIsFromUserInput) {
>      let androidEvents = [];
>  
> +    if (!aIsFromUserInput) {

Could you flip this to be if (aIsFromUserInput) ... ?

@@ +287,5 @@
>    }
>  
>    _infoFromContext(aContext) {
>      let state = Utils.getState(aContext.accessible);
> +    let info = {

nit: const

Comment 6

[Eitan Isaacson [:eeejay]]

Attachment: Use EditText entry in accessible's classNames and use the hint for the label. r=yzen

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: Use EditText entry in accessible's classNames and use the hint for the label. r=yzen

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae9049c5ac63
Use EditText entry in accessible's classNames and use the hint for the label. r=yzen r=jchen

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ae9049c5ac63

Comment 10

[Cosmin Sabou [:CosminS]]

Backed out for gv-junit failures on test.AccessibilityTest.testTextEntryNode

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&fromchange=f017a427c3cbccf397c9bf960a7b84ae5e801cb9&filter-searchStr=Android%204.3%20API16%2B%20debug%20test-android-em-4.3-arm7-api-16%2Fdebug-geckoview-junit-1%20(gv-junit1)&selectedJob=187479943

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=187479943&repo=mozilla-inbound&lineNumber=1470

Backout link: https://hg.mozilla.org/mozilla-central/rev/3edc9c3ae818490ed36b8bfc8ffdfc9e222b41db

[task 2018-07-10T22:47:39.436Z] 22:47:39     INFO -  org.mozilla.geckoview.test |
[task 2018-07-10T22:47:39.436Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=testTextEntryNode
[task 2018-07-10T22:47:39.437Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
[task 2018-07-10T22:47:39.437Z] 22:47:39  WARNING -  TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.AccessibilityTest.testTextEntryNode | status -2
[task 2018-07-10T22:47:39.438Z] 22:47:39     INFO -  TEST-INFO took 10835ms
[task 2018-07-10T22:47:39.438Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2018-07-10T22:47:39.439Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=7
[task 2018-07-10T22:47:39.439Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.ContentDelegateTest
[task 2018-07-10T22:47:39.440Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2018-07-10T22:47:39.440Z] 22:47:39     INFO -  org.mozilla.geckoview.test | org.mozilla.geckoview.test.ContentDelegateTest:
[task 2018-07-10T22:47:39.440Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=88
[task 2018-07-10T22:47:39.441Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=saveAndRestoreState
[task 2018-07-10T22:47:39.441Z] 22:47:39     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: 1

Comment 11

[Eitan Isaacson [:eeejay]]

Attachment: Use EditText entry in accessible's classNames and use the hint for the label. r=yzen

Comment 12

[Eitan Isaacson [:eeejay]]

Oops, forgot to account for older API.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3d1b39a642f
Use EditText entry in accessible's classNames and use the hint for the label. r=yzen r=jchen

Comment 14

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/f3d1b39a642f

Comment 15

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8990469 [details] [diff] [review]
Use EditText entry in accessible's classNames and use the hint for the label. r?yzen r?jchen

Already r+ed more recent one

Comment 16

[Chris Peterson [:cpeterson]]

Eitan, do you consider this bug to be a blocker for the initial Focus+GV release? If so, we should uplift this fix to 62 beta.

Comment 17

[Eitan Isaacson [:eeejay]]

Comment on attachment 8991159 [details] [diff] [review]
Use EditText entry in accessible's classNames and use the hint for the label. r=yzen

Approval Request Comment
[Feature/Bug causing the regression]: WebView parity
[User impact if declined]: Users will not get basic text editing functions in TalkBack
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: 
[Is the change risky?]: Slightly, but only for TalkBack users
[Why is the change risky/not risky?]:
[String changes made/needed]:

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8991159 [details] [diff] [review]
Use EditText entry in accessible's classNames and use the hint for the label. r=yzen

Geckoview-only fix for a11y, let's uplift for beta 9.

Comment 19

[Chris Peterson [:cpeterson]]

[geckoview:klar:p1] because this bug is a Focus+GV blocker.

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/36921dd17b6b