Long-pressing MenuItemActionViews unexpectedly triggers vibration

RESOLVED FIXED in Firefox 40

Status

()

Firefox for Android
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mcomella, Assigned: Jeff Lu, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 40
All
Android
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

(Whiteboard: [lang=java])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Whereas the other menu items, e.g. "New tab" do not.

Note that MenuItemActionViews are the views in the icon views in the toolbar menu, e.g. back, forward, refresh, star, rl, share, ...
(Assignee)

Comment 1

3 years ago
Created attachment 8584321 [details]
Screenshot of tooltip on menu item

This behavior was introduced with bug #943908. A long-click listener gets added to action items,[1] but by default it does nothing.[2]

The question is, what would be the preferred approach to fix this?

A. To eliminate the vibration, we could add a GeckoMenuItem::setLongClickable(boolean) method to enable/disable long-press detection for a given item.

B. We could use the long-press to display a tooltip, which might be helpful for ambiguous buttons.  E.g., the icon for "Remove from reading list" is just a trash can, which could be interpreted many ways.

I have the tooltip working locally (screenshot attached) and could readily submit a patch for that.  Or, it should be pretty straightforward to implement option A, if that's the way we want to go.


[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/menu/GeckoMenu.java#257
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#350
Flags: needinfo?(michael.l.comella)
(Reporter)

Comment 2

3 years ago
(In reply to Jeff Lu from comment #1)
> I have the tooltip working locally (screenshot attached)

Nice!

Let's check with Anthony and see what he wants to do (see comment 1).
Assignee: nobody → jll544
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
Currently, we use long-press on the "back" button to trigger the session history menu (see bug 847435). I like the idea of leveraging this long-press to do something, but I'm not sure about all the implications of using it for a tooltip right now. 

As long-press is kind of like the double-click for touch devices, we have to be careful to be consistent so as not to confuse the user. I'll need to think about this some more.

In the interim, if long-pressing doesn't do anything, then this "unexpected" vibration should not exist. I.e. if there's no function to long-pressing something, there should be no vibration on long-press.
Flags: needinfo?(alam)
(Assignee)

Comment 4

3 years ago
Created attachment 8584849 [details] [diff] [review]
bug-1145252-part1.patch

I can understand the concern about consistency.  I've attached a patch for the vibration on unhandled long-clicks.  The fix is easier than I initially thought; it's simply a matter of passing an already-implemented return result up to the platform.

Try in progress:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=073d1973296b
Attachment #8584849 - Flags: review?(michael.l.comella)
(Reporter)

Comment 5

3 years ago
Comment on attachment 8584849 [details] [diff] [review]
bug-1145252-part1.patch

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

Nice find - thanks, Jeff!

Don't forget to add the "checkin-needed" keyword!
Attachment #8584849 - Flags: review?(michael.l.comella) → review+
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/28d346b0142c
Keywords: checkin-needed
Whiteboard: [lang=java] → [lang=java][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/28d346b0142c
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Whiteboard: [lang=java][fixed-in-fx-team] → [lang=java]
Target Milestone: --- → Firefox 40
You need to log in before you can comment on or make changes to this bug.