1071777 - [AccessFu] Context menus stopped working in Android

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

There has been a change in how context menus are invoked in Firefox for android. Sensing a Gesture:LongPress doesn't work anymore, we need to dispatch a contextmenu event.

Comment 1

[Max Li [:maxli]]

It appears that when you double tap and hold, there's already a context menu event being fired. It seems as though TalkBack dispatches a native long press event when you do the gesture, but we don't process it correctly.

Comment 2

[Max Li [:maxli]]

Attachment: Patch - part 1

So it seems that we reject the context menu event because with Talkback on, there's no highlighting of elements and thus we don't know which element to show a context menu for.

I'm not sure if this is the best approach, but it doesn't seem to cause any regressions with accessibility off. What do you think?

Comment 3

[Max Li [:maxli]]

Attachment: Patch - part 2

Though for braille, we still need to dispatch a context menu event because it instead uses the accessible actions to notify us that something has been long pressed. (This patch also does a little bit of cleanup in removing the Gesture:LongPress code)

I don't have a braille display, but I think this should work (it works if I map the sendContextMenu call to some arbitrary gesture and test it like that). Marco, could you check?

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Presumably this regression is from bug 788073. Max, can you confirm this is still an issue after the landing of bug 1078029?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8519644 [details] [diff] [review]
Patch - part 1

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

This patch looks pretty safe regardless. Thanks!

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Also please request uplift for this to 34 after it lands. Thanks!

Comment 7

[Marco Zehe (:MarcoZ)]

Comment on attachment 8519646 [details] [diff] [review]
Patch - part 2

Thanks! This looks good and works as you suggested. :)

Comment 8

[Max Li [:maxli]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #4)
> Presumably this regression is from bug 788073. Max, can you confirm this is
> still an issue after the landing of bug 1078029?

It does appear that the bug 1078029 patch solves it when you're using talkback and touch gestures, but you  don't seem to have a highlighted element if you're only using a braille device. So I think the first part of the patch is still necessary.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Cool, thanks for checking!

Comment 10

[Eitan Isaacson [:eeejay]]

Comment on attachment 8519646 [details] [diff] [review]
Patch - part 2

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

Code looks good, and Marco tested, so its a go!

Comment 11

[Max Li [:maxli]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d63fe263014c
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a02cfeda9a6

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d63fe263014c
https://hg.mozilla.org/mozilla-central/rev/5a02cfeda9a6

Comment 13

[Max Li [:maxli]]

Comment on attachment 8519644 [details] [diff] [review]
Patch - part 1

Approval Request Comment
[Feature/regressing bug #]: unknown
[User impact if declined]: Users of certain modes of accessibility will be unable to bring up context menus.
[Describe test coverage new/current, TBPL]: manual testing only
[Risks and why]: none known
[String/UUID change made/needed]: none

Comment 14

[Max Li [:maxli]]

Comment on attachment 8519646 [details] [diff] [review]
Patch - part 2

Approval Request Comment
[Feature/regressing bug #]: unknown
[User impact if declined]: Users of certain modes of accessibility will be unable to bring up context menus.
[Describe test coverage new/current, TBPL]: manual testing only
[Risks and why]: none known
[String/UUID change made/needed]: none

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a6db51709467
https://hg.mozilla.org/releases/mozilla-aurora/rev/8e52ea649a53