Closed Bug 1087673 Opened 5 years ago Closed 5 years ago

Vibrate on long back press

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 36

People

(Reporter: bnicholson, Assigned: vivek, Mentored)

References

Details

(Whiteboard: [lang=java])

Attachments

(1 file, 3 obsolete files)

In both Android UI and in-page links, we vibrate when the user long presses an item. We could do the same thing for the phone's back button when showing the history menu.
I would like to take this up as it is related to my current fix for 847435
Depends on: 847435
Assignee: nobody → vivekb.balakrishnan
Attached patch 1087673.patch (obsolete) — Splinter Review
Vibration enabled for 100 milliseconds when showing the tab history.
Attachment #8519425 - Flags: review?(bnicholson)
Comment on attachment 8519425 [details] [diff] [review]
1087673.patch

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

Have you tried unchecking the "vibrate on touch" pref in Settings to verify that it doesn't vibrate? We need to make sure we're honoring the system's haptic feedback pref.

::: mobile/android/base/resources/values/integers.xml
@@ +7,5 @@
>  
>      <integer name="number_of_top_sites">6</integer>
>      <integer name="number_of_top_sites_cols">2</integer>
>      <integer name="max_icon_grid_columns">4</integer>
> +    <integer name="tab_history_vibrate_duration_in_msecond">100</integer>

Nit: Please rename this to long_press_vibrate_msec. Having a generic name is useful if we end up using vibration anywhere else.
Attached patch 1087673.patch (obsolete) — Splinter Review
Review comments addressed.
Android "vibrate on touch" settings honoured.
Attachment #8519425 - Attachment is obsolete: true
Attachment #8519425 - Flags: review?(bnicholson)
Attachment #8520727 - Flags: review?(bnicholson)
Comment on attachment 8520727 [details] [diff] [review]
1087673.patch

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

Thanks!
Attachment #8520727 - Flags: review?(bnicholson) → review+
Keywords: checkin-needed
Don't forget, there are a couple of things that need to be done before we can set checkin-needed:
* The patch should contain the bug #, a summary of what was fixed, and the r=<reviewer> note.
* There should be a link to a try push in the bug.
Flags: needinfo?(vivekb.balakrishnan)
Keywords: checkin-needed
Attached patch 1087673.patch (obsolete) — Splinter Review
patch summary updated

Try run log:
https://tbpl.mozilla.org/?tree=Try&rev=a69c0c6c9b4e
Attachment #8520727 - Attachment is obsolete: true
Flags: needinfo?(vivekb.balakrishnan)
Attachment #8520975 - Flags: review?(bnicholson)
Attachment #8520975 - Flags: review?(bnicholson) → review+
Keywords: checkin-needed
Attachment #8520975 - Attachment is patch: true
Needs rebasing.
Keywords: checkin-needed
Attached patch 1087673.patchSplinter Review
Rebased with latest tree
Keywords: checkin-needed
Comment on attachment 8520975 [details] [diff] [review]
1087673.patch

There's no harm in marking bugs obsolete and it makes the attachment list easier to read :)
Attachment #8520975 - Attachment is obsolete: true
https://hg.mozilla.org/integration/fx-team/rev/95751725d539
Keywords: checkin-needed
Whiteboard: [lang=java] → [lang=java][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/95751725d539
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=java][fixed-in-fx-team] → [lang=java]
Target Milestone: --- → Firefox 36
Depends on: 1100742
You need to log in before you can comment on or make changes to this bug.