Last Comment Bug 1182222 - [AccessFu][Androd] local menu improvements
: [AccessFu][Androd] local menu improvements
Status: RESOLVED FIXED
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: ARM Android
-- normal (vote)
: mozilla43
Assigned To: Eitan Isaacson [:eeejay]
:
: alexander :surkov
Mentors:
Depends on:
Blocks: AndroidA11yUpdates 1203697
  Show dependency treegraph
 
Reported: 2015-07-09 11:44 PDT by Yura Zenevich [:yzen]
Modified: 2015-09-16 08:24 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Make Layerview support accessibility HTML navigation. (15.40 KB, patch)
2015-08-21 13:39 PDT, Eitan Isaacson [:eeejay]
no flags Details | Diff | Splinter Review
Make Layerview support accessibility HTML navigation. (15.40 KB, patch)
2015-08-25 13:31 PDT, Eitan Isaacson [:eeejay]
no flags Details | Diff | Splinter Review
Make Layerview support accessibility HTML navigation. (18.81 KB, patch)
2015-09-10 13:50 PDT, Eitan Isaacson [:eeejay]
yzenevich: review+
mark.finkle: review+
Details | Diff | Splinter Review

Description User image Yura Zenevich [:yzen] 2015-07-09 11:44:06 PDT
Here's a list of potential improvements we can take on:

* line granularity
* section navigation (like landmark)
* list navigation
* control navigation  -> we might be able to do that based on things being actionable
* special content navigation [??]
Comment 1 User image Eitan Isaacson [:eeejay] 2015-08-21 13:39:04 PDT
Created attachment 8651221 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

This patch makes the local context menu identical to Chrome. The behavior is slightly different for the different granularity settings.
* Character/Word/Paragraph act the same as they did before.
* "Control" skips between actionable items, including links.
* "List" skips to the first item on any list on the page. This is instead of putting the cursor on the entire list.
* "Section" skips to landmarks. I think it would be smart if this also went to headings, since that is de-facto how pages are organized when they don't use other kinds of landmarks. WHat do you think?

I'll add a link to a try run for a quick doenload of a build.
Comment 3 User image Marco Zehe (:MarcoZ) 2015-08-24 05:48:36 PDT
When I try to open the build directory for this try build, I get an "artifact not found" error message. :(
Comment 4 User image Marco Zehe (:MarcoZ) 2015-08-25 01:30:00 PDT
Comment on attachment 8651221 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

cancelling feedback until we have a working try build. Tried to build locally, but for some reason the patch doesn't apply cleanly, and afterwards I get build errors.
Comment 5 User image Eitan Isaacson [:eeejay] 2015-08-25 13:31:17 PDT
Created attachment 8652529 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

Rebased.
Comment 6 User image Marco Zehe (:MarcoZ) 2015-08-26 07:43:44 PDT
Comment on attachment 8652529 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

This works partially. The normal navigation works. The Navigation by Control Types also works, but I got neither lists, nor special items, nor sections to work.

Also, if I set TalkBack to show the context menu as a list, when returning to Fennec after dismissing that list, the new setting is not retained, but reset to the default granularity.
Comment 7 User image Marco Zehe (:MarcoZ) 2015-09-09 07:07:41 PDT
Eitan, have you been able to verify my findings from the previous comment? What's the status here?
Comment 8 User image Eitan Isaacson [:eeejay] 2015-09-09 09:38:04 PDT
Indeed! I have incorporated all of your input. Expect a new patch soon.
Comment 9 User image Eitan Isaacson [:eeejay] 2015-09-10 13:50:06 PDT
Created attachment 8659477 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.
Comment 10 User image Marco Zehe (:MarcoZ) 2015-09-11 05:06:52 PDT
Comment on attachment 8659477 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

This patch works as expected! Including showing the local context menu as list.Also the different granularities work as I would expect them to.
Comment 11 User image Yura Zenevich [:yzen] 2015-09-11 10:51:27 PDT
Comment on attachment 8659477 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

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

JS part looks good, see one suggestion

::: accessible/jsat/AccessFu.jsm
@@ +311,4 @@
>        case 'Accessibility:PreviousObject':
> +      {
> +        let rule = aData ?
> +          aData.substr(0, 1).toUpperCase() + aData.substr(1).toLowerCase() :

can also do
aData.toLowerCase().replace(/^./, c => c.toUpperCase())
Comment 12 User image Mark Finkle (:mfinkle) (use needinfo?) 2015-09-11 12:00:11 PDT
Comment on attachment 8659477 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

Looks OK to me.
Comment 14 User image Wes Kocher (:KWierso) 2015-09-14 17:16:41 PDT
https://hg.mozilla.org/mozilla-central/rev/2ed46aca6ae0
Comment 15 User image vortex 2015-09-16 08:24:16 PDT
@Eeejay: I think including headings in section navigattion is a very good idea. Few sites have landmarks, but almost all do have headings....

Note You need to log in before you can comment on or make changes to this bug.