Yura Zenevich [:yzen] (please use "needinfo") Reporter
	Description • 9 years ago

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 [??]

	Eitan Isaacson [:eeejay] Assignee
	Comment 1 • 9 years ago

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.

	Eitan Isaacson [:eeejay] Assignee
	Comment 2 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e95056ba473d

	Marco Zehe (:MarcoZ)
	Comment 3 • 9 years ago

When I try to open the build directory for this try build, I get an "artifact not found" error message. :(

	Marco Zehe (:MarcoZ)
	Comment 4 • 9 years ago

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.

	Eitan Isaacson [:eeejay] Assignee
	Comment 5 • 9 years ago

Rebased.

	Marco Zehe (:MarcoZ)
	Comment 6 • 9 years ago

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.

	Marco Zehe (:MarcoZ)
	Comment 7 • 9 years ago

Eitan, have you been able to verify my findings from the previous comment? What's the status here?

	Eitan Isaacson [:eeejay] Assignee
	Comment 8 • 9 years ago

Indeed! I have incorporated all of your input. Expect a new patch soon.

	Marco Zehe (:MarcoZ)
	Comment 10 • 9 years ago

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.

	Yura Zenevich [:yzen] (please use "needinfo") Reporter
	Comment 11 • 9 years ago

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())

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 12 • 9 years ago

Comment on attachment 8659477 [details] [diff] [review]
Make Layerview support accessibility HTML navigation.

Looks OK to me.

	Pulsebot
	Comment 13 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ed46aca6ae0

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/2ed46aca6ae0

	vortex
	Comment 15 • 9 years ago

@Eeejay: I think including headings in section navigattion is a very good idea. Few sites have landmarks, but almost all do have headings....