[AccessFu] Support editing mode navigation in Jelly Bean

RESOLVED FIXED in mozilla24

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: MarcoZ, Assigned: maxli)

Tracking

Trunk
mozilla24
ARM
Android
Points:
---

Firefox Tracking Flags

(relnote-firefox 24+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

STR:
1. Set focus on an entry anywhere, and type a few characters.
2. Quickly swipe down and up (in one motion). This will set Talkback to navigation by character.
3. Swipe right and left.

Expected: We should navigate through characters that we just typed.
Actual: Only a click is heard as if nothing is in the text field.
Sorry for the spam, this happens if you dogfood a not quite fully accessible nightly build. ;)
Component: XTF → Disability Access APIs
QA Contact: accessibility-apis
This is still an issue, and not even the implementation of editor key bindings in bug 831144 fixed it.
Since edits are native Android widgets even when they come from web pages, what we need to do, I think, is make sure all the AccessibilityNodeInfo properties pertaining to editing gestures are exposed properly:
http://developer.android.com/reference/android/view/accessibility/AccessibilityNodeInfo.html

Specifically, I believe we have to implement the movement granularities for character, word, etc.
Posted patch Patch (obsolete) — Splinter Review
Assignee: nobody → maxli
Attachment #763262 - Flags: feedback?(eitan)
Comment on attachment 763262 [details] [diff] [review]
Patch

I built locally with this patch and noticed that the granularity modes "Words" and "Paragraphs" are still available, but when swiping left and right while either of these is selected, movement by character is still being executed. Can we prevent these two modes from showing up at all? And if not, could we tap into the nsIAccessibleText or other methods to actually move by words?
Comment on attachment 763262 [details] [diff] [review]
Patch

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

I'll defer to Marco's feedback here. If we do this, we either need to remove the paragraph and word option, or support it.
Attachment #763262 - Flags: feedback?(eitan)
It appears that versions of Talkback prior to the current 3.4 beta necessarily display word and paragraph granularities (though the 3.4 beta actually checks what granularities are exposed).
Posted patch Patch v2 (obsolete) — Splinter Review
Attachment #763262 - Attachment is obsolete: true
Attachment #764303 - Flags: review?(eitan)
Comment on attachment 764303 [details] [diff] [review]
Patch v2

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

It looks like this patch works outside of just editing mode. How consistent is it with the current virtual cursor? The original scope of this bug was just to have this supported in editable elements.
(In reply to Eitan Isaacson [:eeejay] from comment #8)
> Comment on attachment 764303 [details] [diff] [review]
> Patch v2
> 
> Review of attachment 764303 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It looks like this patch works outside of just editing mode. How consistent
> is it with the current virtual cursor? The original scope of this bug was
> just to have this supported in editable elements.

It doesn't work outside of editing mode. In particular, note that I check the editState and return early if we're not in editing mode.
(In reply to Max Li [:maxli] from comment #9)
> (In reply to Eitan Isaacson [:eeejay] from comment #8)
> > Comment on attachment 764303 [details] [diff] [review]
> > Patch v2
> > 
> > Review of attachment 764303 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > It looks like this patch works outside of just editing mode. How consistent
> > is it with the current virtual cursor? The original scope of this bug was
> > just to have this supported in editable elements.
> 
> It doesn't work outside of editing mode. In particular, note that I check
> the editState and return early if we're not in editing mode.

Ah, cool. I missed that. Review coming up.
Comment on attachment 764303 [details] [diff] [review]
Patch v2

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

Great patch, can't even find one nit. As we discussed on IRC contenteditable is broken, there should be another bug for that. Flagging kats for java changes.
Attachment #764303 - Flags: review?(eitan)
Attachment #764303 - Flags: review?(bugmail.mozilla)
Attachment #764303 - Flags: review+
Comment on attachment 764303 [details] [diff] [review]
Patch v2

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

r+ with the import fixed.

::: mobile/android/base/GeckoAccessibility.java
@@ +10,5 @@
>  import org.mozilla.gecko.util.UiAsyncTask;
>  
>  import org.json.JSONArray;
>  import org.json.JSONObject;
> +import org.json.JSONException;

Move this import up one line so it's in alphabetical order

@@ +342,2 @@
>                                  // XXX: Self brailling gives this action with a bogus argument instead of an actual click action
> +                                if (arguments.getInt(AccessibilityNodeInfo.ACTION_ARGUMENT_MOVEMENT_GRANULARITY_INT) < 0) {

nit: Might be a good idea to use a temp variable to store the result of this arguments.getInt call because you use it again below and a temp variable would make it more readable. I'll leave this up to you
Attachment #764303 - Flags: review?(bugmail.mozilla) → review+
Carrying forward r+
Attachment #764303 - Attachment is obsolete: true
Keywords: checkin-needed
Max, you have commiter access level 3 now, so you can push this to inbound yourself. Congratulations! :)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/46e6080dcdd4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.