1017294 - 8x25_v1.3_FFOS : Music Fast forward and rewind is not happening when performed long tap on Music Widget in device locked state and from notification tray.

Status: RESOLVED FIXED

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

Description

[Leyla]

8x25_v1.3_FFOS : Music Fast forward and rewind is not happening when performed long tap on Music Widget in device locked state and from notification tray.

preconditions:
Some music files are present in device.

Steps:
1. Play a music from music player.
2. Short press power key to lock the device.
3. Music Player widget is displayed in home screen after device lock.
4. Long tap on Next button to fast forward the music.
5. Long tap on Previous button to rewind the music playback and observe.

6. Also try to fast forward and rewind by long tapping on the button from Music widget present in notificatioon tray.

Actual Result:
Music Fast forward and rewind is not happening when performed long tap on Music Widget in device locked state and from notification tray.

Comment 1

[Tiffanie Shakespeare [:tif]]

NI'ing Jacqueline to confirm if this is a bug or works as designed.

Comment 2

[Preeti Raghunath(:Preeti)]

In triage today it came up as feature request. We will not be fixing in 1.3 and hence backlog.

Comment 3

[Jacqueline Savory [:jsavory] UX]

Yes, the user should be able to fast forward and rewind on the lock screen and notification tray as they do on the music app.

Comment 4

[Sherman Chen [:chens] (inactive)]

Attachment: WIP

Hi Dominic,
As offline discussion, prev/next buttons are not able to receive contextmenu events, I've put WIP patch on github, could you take a look and give me some feedback? thanks.

Comment 5

[Dominic Kuo [:dkuo]]

Comment on attachment 8482045 [details] [review]
WIP

Sherman, the approach looks pretty straightforward and good to me, I don't know why the playback widget couldn't receive the contextmenu event either, it's probably because system app has its own behavior, maybe we can needinfo the system peers? and feel free to ping me again if you have addressed the issues and need the review, thanks!

Comment 6

[Sherman Chen [:chens] (inactive)]

Hi Alive, 

Do you have any idea that contextmenu event is not received on playback widget button? 
I've done some test, contextmenu event can be received on div element, but not no button element.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Sherman Chen [:chens] from comment #6)
> Hi Alive, 
> 
> Do you have any idea that contextmenu event is not received on playback
> widget button? 
> I've done some test, contextmenu event can be received on div element, but
> not no button element.

No idea. Platform issue maybe.

Comment 8

[Sherman Chen [:chens] (inactive)]

Attachment: Pull request

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8501644 [details] [review]
Pull request

I'll let Dominic review this part.
Also flag Greg for lockscreen change.

Comment 10

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8501644 [details] [review]
Pull request

I think we should avoid nested switching case. This is really confused. And, should we create test for media playback widget? Since it's growing and I don't find any unit test (at least I found nothing).

Comment 11

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8501644 [details] [review]
Pull request

Thanks Greg, I've update PR and addressed the comments, would you take a look and give some feedback?

Comment 12

[Sherman Chen [:chens] (inactive)]

Hi Dominic, I have some updates in the PR, could you review the patch? Thanks!

Comment 13

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8501644 [details] [review]
Pull request

feedback+ with one nit.

Comment 14

[Dominic Kuo [:dkuo]]

Comment on attachment 8501644 [details] [review]
Pull request

Sherman, the patch looks okay but I found an major issue which might break the fast-seeking function so let's tweak this patch and I will review it again after you address the issues, please read the github comments, thanks!

Comment 15

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8501644 [details] [review]
Pull request

Hi Dominic, I've updated PR and comments addressed, could you review again? 

Thanks!

Comment 16

[Dominic Kuo [:dkuo]]

Comment on attachment 8501644 [details] [review]
Pull request

Sherman, thanks for addressing those issues and now the patch looks good to me! though there is one possible improvement that, the media_playback.js under system and lockscreen seems exactly the same, so could we merge them into one then put it in the shared/ folder? if so, it can be a followup and please file one bug and let's do it :)

Comment 17

[Sherman Chen [:chens] (inactive)]

Gaia-try: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=a89a39cb39c8

Master: https://github.com/mozilla-b2g/gaia/commit/d5465ee468db890002ba4560c10503da0821c1de

Thanks Domi :)