1100742 - Long back press vibration is too strong

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

The long back press vibration should last as long as the vibration for other long presses, such as the URL bar. It's noticeably stronger than that.

Vivek, do you want to take this? This should just be a matter of testing different durations to find a value that feels consistent. Sorry, I should have double-checked a build before giving r+!

Comment 1

[Vivek Balakrishnan[:vivek]]

Attachment: Long presses vibration intensity reduced

Patch for milder vibration on long press:
* Declared long press duration value similar to that of android framework.

Comment 2

[Brian Nicholson (:bnicholson)]

Comment on attachment 8524887 [details] [diff] [review]
Long presses vibration intensity reduced

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

Just tested this patch and it feels perfect. Just needs some cleanup as described below.

Also, it's usually a good idea to include the bug #, description, and r= in your patch header before requesting review. Then you won't need to post another one after r+!

::: mobile/android/base/GeckoAppShell.java
@@ +1424,5 @@
> +        }
> +        return output;
> +    }
> +
> +

Nit: remove extra newline (although you should remove this method entirely anyway as described below)

@@ +1431,2 @@
>          if (Settings.System.getInt(getContext().getContentResolver(), Settings.System.HAPTIC_FEEDBACK_ENABLED, 0) > 0) {
> +            vibrate(convertIntToLongArray(milliseconds), -1);

Instead of convertIntToLongArray, you should be able to use getLongIntArray: http://androidxref.com/5.0.0_r2/xref/frameworks/base/policy/src/com/android/internal/policy/impl/PhoneWindowManager.java#1084

::: mobile/android/base/resources/values/arrays.xml
@@ +146,5 @@
>      <string-array name="pref_titlebar_mode_values">
>          <item>0</item>
>          <item>1</item>
>      </string-array>
> +    <!-- This value is similar to config_longPressVibePattern in android frameworks/base/core/res/res/values/config.xml-->

similar to -> equal to

Comment 3

[Brian Nicholson (:bnicholson)]

(In reply to Brian Nicholson (:bnicholson) from comment #2)
> Instead of convertIntToLongArray, you should be able to use getLongIntArray:
> http://androidxref.com/5.0.0_r2/xref/frameworks/base/policy/src/com/android/
> internal/policy/impl/PhoneWindowManager.java#1084

Err, ignore this comment. I see now that this method is defined exactly the same way at http://androidxref.com/5.0.0_r2/xref/frameworks/base/policy/src/com/android/internal/policy/impl/PhoneWindowManager.java#5236.

Comment 4

[Vivek Balakrishnan[:vivek]]

Attachment: milder long back press vibration

Review nit corrected

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 8525420 [details] [diff] [review]
milder long back press vibration

Looks great, thanks!

Comment 6

[Vivek Balakrishnan[:vivek]]

Try server logs:
https://tbpl.mozilla.org/?tree=Try&rev=20db79bc1d28

Comment 7

[Carsten Book [:Tomcat]]

Hi,

this patch didn't apply cleanly:

applying file_1100742.txt
patching file mobile/android/base/BrowserApp.java
Hunk #1 FAILED at 523
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/BrowserApp.java.rej
patching file mobile/android/base/GeckoAppShell.java
Hunk #1 FAILED at 1409
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/GeckoAppShell.java.rej
patching file mobile/android/base/resources/values/arrays.xml
Hunk #1 FAILED at 141
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/resources/values/arrays.xml.rej
patching file mobile/android/base/resources/values/integers.xml
Hunk #1 FAILED at 2
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/resources/values/integers.xml.rej

could you take a look thanks!

Comment 8

[Vivek Balakrishnan[:vivek]]

Attachment: 1100742 patch

Patch rebased with latest tree

Comment 9

[Carsten Book [:Tomcat]]

hm still don't apply cleanly

patching file mobile/android/base/BrowserApp.java
Hunk #1 FAILED at 523
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/BrowserApp.java.rej
patching file mobile/android/base/GeckoAppShell.java
Hunk #1 FAILED at 1409
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/GeckoAppShell.java.rej
patching file mobile/android/base/resources/values/arrays.xml
Hunk #1 FAILED at 141
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/resources/values/arrays.xml.rej
patching file mobile/android/base/resources/values/integers.xml
Hunk #1 FAILED at 2
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/resources/values/integers.xml.rej
patch failed, unable to continue (try -v)

does the patch apply for you cleanly ?

Comment 10

[Vivek Balakrishnan[:vivek]]

Attachment: 1100742 patch

A new rebased patch.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I hate to do this, but it's bitrotted again. :(

Comment 12

[Brian Nicholson (:bnicholson)]

Vivek, it looks like the patch hasn't changed any of the times you've tried to unbitrot. Please do "hg pull --rebase" to rebase your patch to tip, then fix any merge conflicts (if there are any). Feel free to ping me if you need more help!

Comment 13

[Vivek Balakrishnan[:vivek]]

Attachment: 1100742.patch

A patch after rebase as per bnicholson's comment.
Hope it works

Comment 14

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/fx-team/rev/64ba905a6180

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/64ba905a6180

Comment 16

[Brian Nicholson (:bnicholson)]

Comment on attachment 8529128 [details] [diff] [review]
1100742.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1087673
[User impact if declined]: The long press back menu vibration is too strong.
[Describe test coverage new/current, TBPL]: Tested locally
[Risks and why]: Very low risk. Simply changes the vibration pattern for the long back press.
[String/UUID change made/needed]: None

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/61638e53ef7b