Closed Bug 1071777 Opened 6 years ago Closed 6 years ago

[AccessFu] Context menus stopped working in Android

Categories

(Core :: Disability Access APIs, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- unaffected
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: eeejay, Assigned: maxli)

References

Details

Attachments

(2 files)

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.
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.
Assignee: nobody → maxli
Attached patch Patch - part 1Splinter Review
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?
Attachment #8519644 - Flags: review?(bugmail.mozilla)
Attached patch Patch - part 2Splinter Review
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?
Attachment #8519646 - Flags: feedback?(mzehe)
Presumably this regression is from bug 788073. Max, can you confirm this is still an issue after the landing of bug 1078029?
Blocks: 788073
Comment on attachment 8519644 [details] [diff] [review]
Patch - part 1

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

This patch looks pretty safe regardless. Thanks!
Attachment #8519644 - Flags: review?(bugmail.mozilla) → review+
Also please request uplift for this to 34 after it lands. Thanks!
Comment on attachment 8519646 [details] [diff] [review]
Patch - part 2

Thanks! This looks good and works as you suggested. :)
Attachment #8519646 - Flags: feedback?(mzehe) → feedback+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
Attachment #8519646 - Flags: review?(eitan)
Cool, thanks for checking!
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!
Attachment #8519646 - Flags: review?(eitan) → review+
https://hg.mozilla.org/mozilla-central/rev/d63fe263014c
https://hg.mozilla.org/mozilla-central/rev/5a02cfeda9a6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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
Attachment #8519644 - Flags: approval-mozilla-aurora?
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
Attachment #8519646 - Flags: approval-mozilla-aurora?
Attachment #8519644 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8519646 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.