973831 - [Music] the player's previous and next buttons should be disabled when music is launched as picker

Status: RESOLVED FIXED

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

Description

[Dominic Kuo [:dkuo]]

This is a regression caused by bug 946556, when the user use music to pick some audio to attach, like attaching audio files in sms, the player's previous and next buttons were disabled intentionally to provide only the basic functionality(play/pause) to preview the selected song, we should fix this or it breaks the state that we don't provide previous and next in music picker.

Comment 1

[Dominic Kuo [:dkuo]]

Attachment: also check MusicComms.enabled when checking SCO status

Jim,

Unfortunately bug 946556 has caused this regression which I didn' notice, but the good news is I found it and the patch is simple. Because when music is in pick mode, the MusicComms object is not enabled, so by also checking MusicComms.enabled, music will not apply the SCO status to enable the previous and next buttons, which we already disabled when the sourceType is set. can you please review this? thanks.

(This also reminds me that, we should probably disable the player when music is called by pick or open activity while SCO is enabled...but that can be separated bugs and not blockers I guess, we can fix that later but that also depends on the new audio competing guideline which ux team is working on)

Comment 2

[Jason Smith [:jsmith]]

What exactly happens when previous & next is selected in this scenario?

Comment 3

[Dominic Kuo [:dkuo]]

(In reply to Jason Smith [:jsmith] from comment #2)
> What exactly happens when previous & next is selected in this scenario?

Previous button: same as the previous button in regular music.
Next button: pause and reset the selected song to the beginning, unable to skip to next track so the user will feel the next button is broken.

Apparently if the previous and next buttons act like this, it confuses the user, also, we don't provide repeat and shuffle in pick mode because the player is used as a previewer, so previous and next are unnecessary, too.

Comment 4

[Preeti Raghunath(:Preeti)]

When previous and next are enabled how does the user recover?

Comment 5

[Jim Porter (:squib)]

Comment on attachment 8378150 [details] [review]
also check MusicComms.enabled when checking SCO status

This seems simple enough; rs=me!

Comment 6

[Dominic Kuo [:dkuo]]

(In reply to Preeti Raghunath(:Preeti) from comment #4)
> When previous and next are enabled how does the user recover?

The user won't be able to recover because the logic of previous and next are not designed for the player in music picker, they will always act like comment 3.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Dominic Kuo [:dkuo] from comment #6)
> (In reply to Preeti Raghunath(:Preeti) from comment #4)
> > When previous and next are enabled how does the user recover?
> 
> The user won't be able to recover because the logic of previous and next are
> not designed for the player in music picker, they will always act like
> comment 3.

The problem triage is having here is that we're not seeing what's bad about this. Can you try to clarify this more?

Comment 8

[Stephany Wilkes]

Trying to understand if this is worth blocking 1.3 on. Flagging Jacqueline.

Comment 9

[Dominic Kuo [:dkuo]]

(In reply to Jason Smith [:jsmith] from comment #7)
> The problem triage is having here is that we're not seeing what's bad about
> this. Can you try to clarify this more?

Probably this video explains better than I do:
http://people.mozilla.org/~dkuo/B2G/Gaia/TestMedia/Music_picker_should_disable_previous_and_next.mp4

Comment 10

[Preeti Raghunath(:Preeti)]

OK, so the pause button doesn't work. The next button clearly doesn't work. The previous button works. From the looks of it, it looks like a blocker.

Comment 11

[Preeti Raghunath(:Preeti)]

Backlog for now.

Comment 12

[Jason Smith [:jsmith]]

(In reply to Preeti Raghunath(:Preeti) from comment #11)
> Backlog for now.

To clarify - we moved this to non-blocking because while we agree this is irrelevant UX being shown, the impact is minor. The user would be able to get out of this situation pretty easily to get back to the song they would like to pick.

Comment 13

[Jacqueline Savory [:jsavory] UX]

I agree that this is not worth blocking on. 
I think ideally, the previous and next buttons would work as they do in the music app and still allow the user to navigate. Dominic - just to clarify, does the patch you are working on do this or will your patch hide the controls?

Comment 14

[Dominic Kuo [:dkuo]]

My patch disable and gary out the previous and next buttons, will fix this regression. Those two buttons were disabled because we tried to sync the behaviours for gallery, video and music when they are launched as picker. Our original idea is the mission for the pickers should be simple, just providing preview and pick the attachments with basic functionalities, like video picker, the users have to back to the list then select/preview the video they want. And because music app has previous, next, repeat and shuffle in the player, since repeat and shuffle were disabled we also disabled previous and next because these functionalities are related.

And if we want to let the buttons work as the regular music, I think we should fix this one and file another bug to enable the buttons with correct previous/next logic.

Comment 15

[Dominic Kuo [:dkuo]]

Landed on master: abdabccec260237674e6503e55e10623124a69cc

Comment 18

[jingmei.zhang]

[Blocking Requested - why for this release]:

Comment 19

[Hema Koka [:hema]]

NI Vance who is triaging 1.4 issues

Comment 20

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Minor defect, would suggest partners to cherry pick this one if they really want this fix.

Hi Jinemei, it is just a one-line fix, please cheery-pick it to your branch if you really want it

Thanks

Vance