User Story: As a user while playing music on my phone and the homescreen is locked, I want to access music player controls without having to unlock the screen

Dominic: I've already started on parts of this, and I believe Dave Hylands was going to work with me on this.

Sorry for my jumping in without invitation to this issue. I am heard that we'll introduce mozSettings abuse in this prototype, and as you know this is a really sad hack. I understand (1) the timeframe issue and (2) there're already some abuse of mozSettings in current b2g. My thoughts: * We already killed somebody doesn't mean we should kill another ;) * Personally I really ran into many troubles from the idea: 'It just works'. * We could still work on a long term solution in parallel with the gecko team. I think CJ's team would provide help here, either to fulfill David's original MediaControl design or enhance it to meet all UX requirement we have. Discussed w/ him yesterday, he intended not to use a generic communication API but make a specific one. Anyway, any of these is better than abuse mozSettings. * There maybe some problems about the music app is crashed w/o updating mozSettings. With this we couldn't avoid to monitor 'protocol://music.gaia-mobile.org/'s life cycle in system app by hard coding the origin of music app. * Pay attention to visibilitychange manipulation "in system app". "on lockscreen" and "on utility-tray". So may I know what's the long term plan if we couldn't avoid that? Sorry again to disturb you guys w/o invitation.

Also, have music app keep setting R/W permission for this reason is really sad..

After talking with Hema, I think the plan right now is that I'll implement this using the Settings API, but I *won't* land it until we have a proper API I can replace it with. I think the generic inter-app comms API is the one we want. I can't recall all the details of the media control API, but one issue I had was that I think we should be sending all the commands directly to the music app. This is especially important when hitting next/previous track when the music app is set to shuffle. (The shuffle logic should be handled by the music app, in part so that alternate music apps can invent their own shuffling rules.) A generic inter-app comms API satisfies all our needs there, and has the added bonus of being useful for other apps too.

(In reply to Dominic Kuo [:dkuo] from comment #6) > Created attachment 783632 [details] > Music Player 1.2 release spec., v0.2 Hm, it seems I don't have access to this file. I had this issue before, and never got a clear answer on whether I should just create a box account or if I need to talk to someone to get an account with the right privileges...

Just a quick update on where I'm at: I've gotten the lockscreen UI to look pretty close to the final UI specs, though there are still a few kinks to work out. Most importantly, if anyone knows a lot about the behavior of the lockscreen's elastic "bounce", I could use some pointers for how best to totally disable it when the playback controls are present.

Mainly a note to myself, but I/we will need to investigate this issue: when you pause a track in the music app, then go to the lockscreen and try to hit play, it doesn't work, and the following error is logged in logcat: W/AudioFlinger( 2831): Thread AudioOut_1 cannot connect to the power manager service This looks like some low-level Android stuff, so I don't know what to do about it, but I'm guessing the fix isn't in Javascript-land!

c.c. Yuren for c8

c.c. mchen for c9, he is the guy knows android audio well. But I bet you have better to provide gaia source code.

(In reply to Jim Porter (:squib) from comment #9) > Mainly a note to myself, but I/we will need to investigate this issue: when > you pause a track in the music app, then go to the lockscreen and try to hit > play, it doesn't work, and the following error is logged in logcat: > > W/AudioFlinger( 2831): Thread AudioOut_1 cannot connect to the power > manager service > > This looks like some low-level Android stuff, so I don't know what to do > about it, but I'm guessing the fix isn't in Javascript-land! OOC, I guess you know the rule of audio competing / background play?

you can use LockScreen.setElasticEnabled() to disable it.

(In reply to Alive Kuo [:alive] from comment #12) > (In reply to Jim Porter (:squib) from comment #9) > > W/AudioFlinger( 2831): Thread AudioOut_1 cannot connect to the power > > manager service > > > OOC, I guess you know the rule of audio competing / background play? No, I don't. Do you have a source for that which explains it? And preferably, how to fix it? :) (In reply to Yuren Ju [:yurenju] from comment #13) > you can use LockScreen.setElasticEnabled() to disable it. Sorry, I forgot to update that I'd fixed this issue, but for posterity, setElasticEnabled wasn't enough for me, since other events would re-enable it. I had to add a setElasticAllowed method.

Oh, if you'd like to look at the source code for this branch, you can find it here: https://github.com/jimporter/gaia/tree/music-lockscreen

(In reply to Jim Porter (:squib) from comment #14) > (In reply to Alive Kuo [:alive] from comment #12) > > (In reply to Jim Porter (:squib) from comment #9) > > > W/AudioFlinger( 2831): Thread AudioOut_1 cannot connect to the power > > > manager service > > > > > OOC, I guess you know the rule of audio competing / background play? > > No, I don't. Do you have a source for that which explains it? And > preferably, how to fix it? :) > https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.gaia/HyAlHdFdX68 https://wiki.mozilla.org/WebAPI/AudioChannels But I look into your code and it's still using content channel...so I don't know what happens. Maybe mchen could offer help.

(In reply to Jim Porter (:squib) from comment #9) > W/AudioFlinger( 2831): Thread AudioOut_1 cannot connect to the power > manager service > > This looks like some low-level Android stuff, so I don't know what to do > about it, but I'm guessing the fix isn't in Javascript-land! This log is always there because we didn't have power manager from Android. So this is a normal message not a issue.

Hmm, strange. Do you have any idea why starting playback on a paused audio file when the app in question is in the background? This is an area I'm not really familiar with.

I'm trying to use your repo to look into this.

Patryk: I don't know if Rob MacDonald mentioned this to you, but at the Media Team work week, he took a look at my prototype of the lockscreen media controls and noticed that the buttons are a bit too small. I was following your UI mockup and just used the music app's icons for the buttons. I believe Rob's goal is to make the buttons nice and big so that they're easy to hit if you're out running and just want to go to the next song, for instance. Hopefully I'm getting all this right, since Rob is out this week. Could you take a look at this and update the UI mockups? I think we could either 1) make new, bigger icons, or 2) just make the hit areas larger (like how the camera and unlock buttons are actually bigger than their icons).

Yes Rob and I spoke, I sent him the controls but maybe they got lost in his inbox. Try these.

https://moztrap.mozilla.org/manage/caseversion/69966/ is in draft form to cover the basic testing of this feature.

As discussed, moving this feature to 1.3

This isn't done yet, but I want to make sure I'm going in the right direction with the system changes. What do you think so far, Alive?

Comment on attachment 812870 [details] https://github.com/mozilla-b2g/gaia/pull/12596 Jim, this is awesome! I love OO! BTW I'm worried about the way using static method and static attribute on the class object. Please contact me if you have problems on what I said. Thanks.

Ok, I've tested this out a bunch now (going on walks while listening to music; stress-testing the lockscreen stuff by turning the screen on, putting the phone in my pocket, and dancing around like an idiot), and I think hiding the playback controls until you tap on the "now playing" info is sufficient to prevent most accidental clicks. I actually found it easier to accidentally unlock the phone than to accidentally hit the controls. I still need to make the UI for hiding the controls a bit prettier, since I was initially just doing a quick-and-dirty solution to test the functionality, but that shouldn't take too long.

Quick update: bug 911681 is going to bitrot me moderately badly, although the long-term effect will be significantly simpler code for this stuff, so yay!

Comment on attachment 812870 [details] https://github.com/mozilla-b2g/gaia/pull/12596 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. Ok, I think this is done. Alive: Most of the changes are in the system app, naturally. Now that bug 911681 is landed, we use DOM events to distribute IAC messages throughout the system app. I think this ends up making the code a lot simpler, so hopefully you'll approve. Rob: Could you take a look at the UI for this? I'll try to post screenshots and send you a short video tonight or this weekend. EragonJ: I made a couple of minor changes to the iac_handler.js file. Could you take a quick look at them to make sure everything is ok? Gareth: Could you look over the marionette tests? I'm getting random oranges on the "should hide now playing info by exiting" test, and I'm not sure what's causing it.

Comment on attachment 812870 [details] https://github.com/mozilla-b2g/gaia/pull/12596 Oops, I didn't mean to request v1.2 approval!

Comment on attachment 812870 [details] https://github.com/mozilla-b2g/gaia/pull/12596 Hey Jim -- I've been playing around with your patch for a while and haven't been able to reproduce the failure :(. The test looks really good to me. Could you add a bit more info about the orange... perhaps a stacktrace? On a brighter note, I really like some of the composition/coding style here :). Also, would you mind renaming or moving MediaPlaybackTest? When I see a class named SomeClassName I usually expect to find it in something like some_class_name.js, some-class-name.js, etc.

(In reply to Gareth Aye [:gaye] from comment #30) > Comment on attachment 812870 [details] > https://github.com/mozilla-b2g/gaia/pull/12596 > > Hey Jim -- I've been playing around with your patch for a while and haven't > been able to reproduce the failure :(. The test looks really good to me. > Could you add a bit more info about the orange... perhaps a stacktrace? Here's the stack trace: <https://pastebin.mozilla.org/3408934>. It's entirely possible that it's just some weirdness on my end, since I saw these oranges on master too, and Travis doesn't seem to be complaining. > On a brighter note, I really like some of the composition/coding style here > :). Thanks! > Also, would you mind renaming or moving MediaPlaybackTest? When I see a > class named SomeClassName I usually expect to find it in something like > some_class_name.js, some-class-name.js, etc. Will do.

Comment on attachment 812870 [details] https://github.com/mozilla-b2g/gaia/pull/12596 Hi Jim, it looks good to me. Thanks for helping me changing the related part in FTU. :P

Comment on attachment 812870 [details] https://github.com/mozilla-b2g/gaia/pull/12596 Redirecting ui-review? to Jacqueline. See the above attachments for the patch in action. Thanks in advance!

Jacqueline is working on this today.

Hi Jim, I reviewed the screenshots that you have attached and everything looks great. Thank you!

Comment on attachment 812870 [details] https://github.com/mozilla-b2g/gaia/pull/12596 Setting ui-review+ per comment 37. Thanks!

Landed: https://github.com/mozilla-b2g/gaia/pull/12596 https://github.com/mozilla-b2g/gaia/commit/bca3aa71ae7c93f50a99b38911f06462f3219b10

#10964 added to Moztrap to cover this case.

1.3+ for uplift.

Does this really need uplifting? I thought this was already in 1.3, since I landed it a month ago.

Yeah, you can search the hash : 6aa6430356162bf477a003078793a490f8af2c8a on v1.3