Closed Bug 834148 Opened 12 years ago Closed 12 years ago

Volume UP key skips 'vibrate' when moving up (from 'silent' to 'volume 1')

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::System
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:leo+, b2g18+ verified)

Status:
VERIFIED FIXED
Project Flags:
blocking-b2g leo+
Tracking Flags:
Tracking Status
b2g18 + verified

People

(Reporter: leo.bugzilla.gaia, Assigned: leo.bugzilla.gaia)

References

Details

(Whiteboard: leorun3)

Attachments

(3 files, 2 obsolete files)

video - vibrate on when volume > 0
4.58 MB, video/3gpp
Details
State transition table of the video attached
28.90 KB, image/jpeg
Details
Pull request url
198 bytes, text/html
alive
: review+
Details
1. Title : Volume UP key skips 'vibrate' when moving up (from 'silent' to 'volume 1'). 2. Precondition : Press the volume DOWN key until it goes to 'silent'. 3. Tester's Action : Press the volume UP key. 4. Detailed Symptom (ENG.) : The volume is changed to 'volume 1', instead of 'vibrate'. 5. Expected : The volume is expected to be 'silent' => 'vibrate' => 'volume 1', when volume up. (The exact opposite way of volume down) 6.Reproducibility: Y 1)Frequency Rate : 100% 7.Gaia Revision: a03f7b532e9998e646d55f93a0fc03a04d7ca7d9
I have added a proposed patch for this issue. Please find the pointer for pull request at https://github.com/mozilla-b2g/gaia/pull/8196. Please review the patch.
Assignee: nobody → leo.bugzilla.gaia
Attached patch Patch URL to pull request 8196 (obsolete) — — Splinter Review
Attachment #715874 - Flags: review?
Comment on attachment 715874 [details] [diff] [review] Patch URL to pull request 8196 ><html> > <head> > <meta http-equiv="Refresh" > content="2; url=https://github.com/mozilla-b2g/gaia/pull/8196" /> > </head> > <body> > Redirect to pull request #8196 > </body> ></html>
Attachment #715874 - Flags: review? → review?(timdream)
Comment on attachment 715874 [details] [diff] [review] Patch URL to pull request 8196 redirecting review. Hi anonymous contributor!
Attachment #715874 - Flags: review?(timdream) → review?(alive)
Sorry I didn't read the code because this may not be expected. c.c. UX to confirm the behavior.
Flags: needinfo?(kyee)
Comment on attachment 715874 [details] [diff] [review] Patch URL to pull request 8196 I don't think so. Volume key going down: volume+ => vibrate on+no volume => vibrate off+no volume Volume key going up: We should either ignore the vibrate state or restore the vibrate state(no v1) instead of adding a state between silent and non-silent state.
Attachment #715874 - Flags: review?(alive)
(In reply to leo.bugzilla.gaia from comment #0) > 5. Expected : The volume is expected to be 'silent' => 'vibrate' => 'volume > 1', when volume up. (The exact opposite way of volume down) The expected behavior here is correct.
Flags: needinfo?(kyee)
Attachment #715874 - Flags: review?(alive)
Somehow the review flag is removed. Since it is confirmed by UX, I have set the review flag again.
Comment on attachment 715874 [details] [diff] [review] Patch URL to pull request 8196 r=me w/ nit address and if you squash the commits. One bug one commit if possible. Thanks.
Attachment #715874 - Flags: review?(alive) → review+
Casey, it's little strange that we would have an extra step to let volume up. I still don't think this is right...
Need info again. Final chance ;)
Flags: needinfo?(kyee)
I think it makes sense that some users will want to change their phone from silent to vibrate. If we make it asymmetrical the user would have to volume up then back down again to activate vibrate. That's my take on it and I'm open to hear why you think otherwise. :)
Flags: needinfo?(kyee)
In my personal experience, all smart/feature phones I've had behaved the same way (change from silent to vibrate). :)
blocking-b2g: --- → leo?
blocking-b2g: leo? → -
tracking-b2g18: --- → +
Not critical enough to block, but definitely want to include in 1.1, so tracking+.
I thought this was by design? Because if you go down w/ vibrate turned off it will automatically turn vibrate on. If you go down all the way, it will turn off vibrate. This will allow you to control the vibrate on/off without having to explicitly turn it off in the settings each time. If we don't allow for the skip, the end user will have to turn off the vibrate each time even though the sound is on.
Note: iphone have a separate hardware switch for vibrate. We don't have that. On Galaxy S II, you can't control vibrate through volume button.
please see comment 15 and 16. I think we need asymmetry in terms of usability and controlling vibrate using the hardware volume buttons.
Flags: needinfo?(kyee)
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #17) > please see comment 15 and 16. I think we need asymmetry in terms of > usability and controlling vibrate using the hardware volume buttons. I agree. I think this needs to be nominated higher priority.
Flags: needinfo?(kyee)
Flags: needinfo?(fdjabri)
(In reply to Leo from comment #18) > (In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from > comment #17) > > please see comment 15 and 16. I think we need asymmetry in terms of > > usability and controlling vibrate using the hardware volume buttons. > > I agree. I think this needs to be nominated higher priority. I also agree. We should bring the user out the same way we brought them in.
Flags: needinfo?(fdjabri)
The following table shows the volume/vibrate/mute status after pressing up/down (↑/↑). This is the same transition from the video attached. Notice there are states where volume is greater than 0 and have the vibrate on. Press ↓ ↓ ↓ ↑ ↑ ↑ ↓ ↓ ↓ ↓ ↑ ↑ ↑ -------------------------------------------------------------------------------------------------------------- Volume | 3 2 1 0 1 2 3 2 1 0 0 1 2 3 Vibrate | off off off on on on on on on on off off off off Mute | - - - - - - - - - - on - - - This is because the current changeVolume function only changes the vibrate status when the mute state is on. switch (muteState) { ... case 'MUTE': classes.add('mute'); if (channel == 'notification') { if (vibrationEnabled) { classes.add('vibration'); SettingsListener.getSettingsLock().set({ 'vibration.enabled': true }); } else { classes.remove('vibration'); SettingsListener.getSettingsLock().set({ 'vibration.enabled': false }); } } break; } The pull request in this bug fixes this issue as well. https://bugzilla.mozilla.org/attachment.cgi?id=715874&action=edit Alive, how about I revise the pull request per your comment and merge this PR to V1-train?
Attachment #756371 - Flags: feedback?(alive)
The above state transition table in Text is not aligned well. Let me attach it as a screenshot.
Adding a row for the Mute state to the transition table.
Attachment #756372 - Attachment is obsolete: true
(In reply to Leo from comment #20) > Created attachment 756371 [details] > video - vibrate on when volume > 0 > > The following table shows the volume/vibrate/mute status after pressing > up/down (↑/↑). > This is the same transition from the video attached. > > Notice there are states where volume is greater than 0 and have the vibrate > on. > > > Press ↓ ↓ ↓ ↑ ↑ ↑ ↓ ↓ ↓ ↓ ↑ > ↑ ↑ > ----------------------------------------------------------------------------- > --------------------------------- > Volume | 3 2 1 0 1 2 3 2 1 0 0 > 1 2 3 > Vibrate | off off off on on on on on on on off > off off off > Mute | - - - - - - - - - > - on - - - > > This is because the current changeVolume function only changes the vibrate > status when the mute state is on. > > switch (muteState) { > ... > case 'MUTE': > classes.add('mute'); > if (channel == 'notification') { > if (vibrationEnabled) { > classes.add('vibration'); > SettingsListener.getSettingsLock().set({ > 'vibration.enabled': true > }); > } else { > classes.remove('vibration'); > SettingsListener.getSettingsLock().set({ > 'vibration.enabled': false > }); > } > } > break; > } > > The pull request in this bug fixes this issue as well. > https://bugzilla.mozilla.org/attachment.cgi?id=715874&action=edit > > Alive, how about I revise the pull request per your comment and merge this > PR to V1-train? If I don't misunderstand something, you could ask uplift or just leo+ this one, if you want this to goto v1-train. Sorry for late response.
(In reply to Alive Kuo [:alive] from comment #23) Should I also put 'checkin-needed' in Keyword to get this into v1-train?
blocking-b2g: - → leo+
(In reply to Leo from comment #24) > (In reply to Alive Kuo [:alive] from comment #23) > > Should I also put 'checkin-needed' in Keyword to get this into v1-train? status-b2g18 ----> affected
status-b2g18: --- → affected
Attachment #756371 - Flags: feedback?(alive)
(In reply to Alive Kuo [:alive] from comment #25) Is this patch ready to get into v1-train?
Could you rebase the commits and then I could merge it? Sorry, I didn't notice it not being merged..
Flags: needinfo?(hanj.kim25)
Attached file Pull request url —
Attachment #762558 - Flags: review?(alive)
Flags: needinfo?(hanj.kim25)
Attachment #715874 - Attachment is obsolete: true
Attachment #762558 - Attachment mime type: text/plain → text/html
Comment on attachment 762558 [details] Pull request url feedback+, thanks. But let's cleanly separate the two functions: getVolume to decide the volume only, and getVolumeState to decide vibration + mute state only.
Comment on attachment 762558 [details] Pull request url See last comment
Attachment #762558 - Flags: review?(alive)
Comment on attachment 762558 [details] Pull request url r=me. Thanks!
Attachment #762558 - Flags: review+
issue repros on Leo Build ID: 20130610070206 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8e3f39363c54 Gaia: ce3b99781d182ad550a325206990c249b0dbcf0e Platform Version: 18.0 when turning the volume up and also down vibrate is skipped and goes directing to volume 1 or silent
Whiteboard: leorun3
(In reply to Alive Kuo [:alive] from comment #30) > Comment on attachment 762558 [details] > Pull request url > > See last comment The pull request has been updated per your comment. Please get them into master/v1-train. Thanks!
Flags: needinfo?(alive)
master https://github.com/mozilla-b2g/gaia/commit/a91a0e4970a11e373eff1b284adbeb320cc3a7a8
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: needinfo?(alive)
Resolution: --- → FIXED
Uplifted a91a0e4970a11e373eff1b284adbeb320cc3a7a8 to: v1-train: 84fb8c5c907ecaa789d1d345fcc5416205e10841
status-b2g18: affected → fixed
When the volume is increased and decreased in can be set in vibrate, mute and to volume 1. The defect does not reproduce. Verified on LEO Build ID: 20130625070217 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/29933d1937db Gaia: 1436e2778b90bd74635b0b94d1cf8ccb0d71b60c Platform Version: 18.1
Status: RESOLVED → VERIFIED
status-b2g18: fixed → verified
See Also: → 908546
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: