Closed Bug 951024 Opened 7 years ago Closed 7 years ago

No haptic feedback/vibration on text selection

Categories

(Firefox for Android :: Text Selection, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: veeti.paananen, Assigned: veeti.paananen)

Details

(Whiteboard: [parity-chrome])

Attachments

(1 file, 3 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.63 Safari/537.36

Steps to reproduce:

Select text


Actual results:

No vibration on selection start


Expected results:

A slight vibration should occur as with native text areas and fields
Attached patch patch.diff (obsolete) — Splinter Review
Attachment #8348507 - Flags: review?(wjohnston)
I see Chrome does this.
Assignee: nobody → veeti.paananen
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Linux → Android
Hardware: x86_64 → ARM
Whiteboard: [parity-chrome]
Comment on attachment 8348507 [details] [diff] [review]
patch.diff

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

::: mobile/android/base/TextSelection.java
@@ +128,5 @@
>                          if (mActionModeTimerTask != null)
>                              mActionModeTimerTask.cancel();
>                          showActionMode(message.getJSONArray("actions"));
> +
> +                        mVibrator.vibrate(20);

Lets just use the code we have in GeckoAppShell here:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoAppShell.java#1438

i.e. you can make this just a one line change:

GeckoAppShell.performHapticFeedback(true);
Attachment #8348507 - Flags: review?(wjohnston) → review+
Attached patch 160705.diff (obsolete) — Splinter Review
Attachment #8348507 - Attachment is obsolete: true
Attached patch 160705.diff (obsolete) — Splinter Review
Oops.
Attachment #8350155 - Attachment is obsolete: true
Attachment #8350156 - Flags: review?(wjohnston)
Sorry for the delay. Looked into this more. This will trigger even if we're just showing the single select handle. Do we want it then? I can't seem to make this happen on my phone in any apps to test, so I'd just like some detail before we land.
Sorry, but what is a "single select handle" and where is it used?
Sorry. thats just words I make up in my head :)

I was trying to describe the little arrow that shows up when you tap in a textbox that already has text in it and points to where the blinking cursor is. We show the action mode whenever that little arrow is shown as well..
(In reply to Wesley Johnston (:wesj) from comment #8)
> tap in a textbox that already has text in it

err... I meant "tap in a textbox that is already focused (i.e. tap in a text box twice)"
Oh, I see. I don't think that's desirable and it definitely doesn't match the native behavior. How's this?
Attached patch 951024.patchSplinter Review
Attachment #8350156 - Attachment is obsolete: true
Attachment #8350156 - Flags: review?(wjohnston)
Attachment #8356594 - Flags: review?(wjohnston)
Comment on attachment 8356594 [details] [diff] [review]
951024.patch

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

Thanks :)
Attachment #8356594 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/integration/fx-team/rev/d8c00c658654
Keywords: checkin-needed
Whiteboard: [parity-chrome] → [parity-chrome][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d8c00c658654
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [parity-chrome][fixed-in-fx-team] → [parity-chrome]
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.