Naoki Hirata :nhirata (please use needinfo instead of cc) Reporter
	Description • 12 years ago

## Environment :
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/f2460bf17811
Gaia   7258384661345ded5a8813c34774ef8a0b56cc02
BuildID 20130114230201
Version 18.0
Otoro

## Repro :
1. load some songs
2. go to Music app
3. tap the far right icon : it's album view...
4. tap the second from the right icon: it's artists...
5. tap the second from the left icon : it's playlist, but when you play a song it shows you just the song
6. tap the far left icon: it's another album view with pictures
7. play a song from any of the views...
8. they all show you just the song.

## Expected :
1. There should be a song list view

## Actual :
1. there is none

## Note :
1. The reason why I request this is sometimes I like to skip to certain songs through the song list.

	Dominic Kuo [:dkuo] Assignee
	Comment 1 • 12 years ago

I have discussed this with Patryk before, and this might need a new UI to achieve this.

	Patryk Adamczyk [:patryk] UX
	Comment 2 • 12 years ago

This should be a v.1.1 feature.

	Casey Yee
	Comment 3 • 12 years ago

Are we referring to playlist here?   If so this has been marked OOS for V1.

	Eric Pang [:epang] UX
	Comment 4 • 12 years ago

Mock up of the song list view.  When the song list tab is selected it works similarly to album view, but filters alphabetically by song title (and artist underneath).

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 6 • 12 years ago

Minusing for tracking based on Casey's comment 3. Renominate if this changes.

	Casey Yee
	Comment 7 • 12 years ago

Sorry, my comment in 3 refers to playlist functionality when in fact the bug is asking for a song list view.

Discussing this with Rob MacDonald, we feel that this is an important feature that we should have for KOI release.

Nominating appropriately.

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 8 • 12 years ago

Mass edit to set tracking-b2g18+ for these UX bugs that were called out for v1.1

	Eric Pang [:epang] UX
	Comment 9 • 12 years ago

Hi Dominic, what's the status of this bug?  We are wrapping up bugs visual design bugs for v1.1.  Do you think this can be ready for review before the end of this week?

	Dominic Kuo [:dkuo] Assignee
	Comment 10 • 12 years ago

Eric, sorry for the late reply, I will start to implement this bug tomorrow, it should be okay to be ready for review before the end of this week, thanks.

	Dominic Kuo [:dkuo] Assignee
	Comment 11 • 12 years ago

David,

you can see this bug might be a koi+ bug, but due to the strongly request from visual team and I also think it's nice to have this in v1.1, so I picked it up and implemented it quickly. Fortunately, this resulted in a simple patch and should be easy to review, would you please review this one? thanks.

	David Flanagan [:djf]
	Comment 12 • 12 years ago

Comment on attachment 733863 [details]
implement all songs view

r+, but see my comment on github.

Also:

1) I haven't tested very well. It looks like there is a gecko bug causing an error that prevents music from playing. Every time I try to play a song (with or without this patch) I get:

E/GeckoConsole( 4975): [JavaScript Warning: "HTTP "Content-Type" of "image/vnd.wap.wbmp" is not supported. Load of media resource blob:ab89702c-a756-4bd3-87bc-340ca9dce5c8 failed." {file: "app://music.gaiamobile.org/index.html" line: 0}

2) Scrolling in the songs list feels slow to me. The code works, but I would expect performance bugs to be filed that you'll have to work on. As a general thing, I think you're going to have to move to a model where you query all the songs from the db once at startup, and keep the list in memory so that you don't have to do any other db operations after starting. Also, trying to switch album/artist/song tabs while the app was scanning new music didn't really work. I think the database queries were taking a really long time, so I was just getting a blank screen.

	Sotaro Ikeda [:sotaro]
	Comment 13 • 12 years ago

(In reply to David Flanagan [:djf] from comment #12)
> Comment on attachment 733863 [details]
> implement all songs view
> 
> r+, but see my comment on github.
> 
> Also:
> 
> 1) I haven't tested very well. It looks like there is a gecko bug causing an
> error that prevents music from playing. Every time I try to play a song
> (with or without this patch) I get:
> 
> E/GeckoConsole( 4975): [JavaScript Warning: "HTTP "Content-Type" of
> "image/vnd.wap.wbmp" is not supported. Load of media resource
> blob:ab89702c-a756-4bd3-87bc-340ca9dce5c8 failed." {file:
> "app://music.gaiamobile.org/index.html" line: 0}
> 

It is Bug 857831.

	Dominic Kuo [:dkuo] Assignee
	Comment 14 • 12 years ago

Thanks David, I have also noticed the problem in 2) and file a new issue in bug 860153. I will land this patch first cause bug 849766 also needs this, then fix the performance issue later.

	Dominic Kuo [:dkuo] Assignee
	Comment 15 • 12 years ago

Landed on master: f0ab15ca1a583080693b27be0d369d12e7dbd133

	Dominic Kuo [:dkuo] Assignee
	Comment 16 • 12 years ago

This bug blocks bug 849766(leo+), so I think we also need to re-nominate this to avoid the uplift lack if the patch of bug 849766 lands.

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 17 • 12 years ago

Seems like this is actually additional functionality once bug 849766 lands and not in itself a blocker, it shouldn't land until after song pick activity is implemented so we'll track and the fix can be nominated for uplift approval consideration when the feature is ready.

	Eric Pang [:epang] UX
	Comment 18 • 12 years ago

(In reply to Dominic Kuo [:dkuo] from comment #15)
> Landed on master: f0ab15ca1a583080693b27be0d369d12e7dbd133

Thanks for working on this Dominic!

	Naoki Hirata :nhirata (please use needinfo instead of cc) Reporter
	Comment 19 • 12 years ago

I'll mark this verified; there's another bug that I would like to add: the current song is not listed in this view.  

Having the current song shown and a way to ff/rewind/pause/stop would be good within this list; so you know which song is next and which song was the previous song.

Master build :2013-04-22-03-05-57
"mozilla-central" revision="50d25e083421"
"integration/gaia-central" revision="6693344385fc"
"gecko.git" revision="12a533fa7b8fa09df333a0ca7041e9f467cb3f4a"
"gaia.git" revision="0874b5e84129391de9edb23560be8ad52b3d2bed"
Unagi

	Naoki Hirata :nhirata (please use needinfo instead of cc) Reporter
	Comment 20 • 12 years ago

Oh looks like Casey already has one : bug 862706

	Sam Joch [:samjoch]
	Comment 21 • 12 years ago

Dominic: can you ask for an approval-gaia-v1 please ?

	Dominic Kuo [:dkuo] Assignee
	Comment 22 • 12 years ago

Comment on attachment 733863 [details]
implement all songs view

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):
String or UUID changes made by this patch:

	Alex Keybl [:akeybl]
	Comment 23 • 12 years ago

Comment on attachment 733863 [details]
implement all songs view

Please fill out the form.

	Dominic Kuo [:dkuo] Assignee
	Comment 24 • 12 years ago

Comment on attachment 733863 [details]
implement all songs view

Sorry about that I didn't notice there is a form needed to be filled out.

========================================================================

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This is a feature request from visual team, please also see the previous comments.
User impact if declined: Users won't be able to see all the songs that displays every tracks on sd card.
Testing completed: Tested locally on a unagi device, landed on master.
Risk to taking this patch (and alternatives if risky): Low, we already have album, artist and playlist list views. The songs list view is another sorting of all the tracks.
String or UUID changes made by this patch: One new string is added - "Songs"

And this bug blocks bug 849766 due to some changes are needed, please also estimate this, thanks.

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 25 • 12 years ago

Is there a way to do this and re-use an existing string instead of adding a new one?

	Axel Hecht [:Pike]
	Comment 26 • 12 years ago

I don't think this works on a per-bug basis. Can we get one bug with one patch to uplift to v1-train? That's because this bug adds a string, as does bug 849766, so and looking at the approval request, discussing the string impact of this bug without the other doesn't make sense.

	Dominic Kuo [:dkuo] Assignee
	Comment 27 • 12 years ago

The strings are not freezed yet for leo+ but tef+, and this patch is landed on master and maybe will be landed on v1-train, I think we don't have to worry about the new string in this patch for requesting approval-gaia-v1.

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 28 • 12 years ago

Comment on attachment 733863 [details]
implement all songs view

Since we're not actually string frozen on v1.1, approving.

	John Ford [:jhford] CET/CEST Berlin Time
	Comment 29 • 12 years ago

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 f0ab15ca1a583080693b27be0d369d12e7dbd133
  <RESOLVE MERGE CONFLICTS>
  git commit

	Eric Pang [:epang] UX
	Comment 30 • 12 years ago

(In reply to John Ford [:jhford] -- If you expect a reply from me, use needsinfo? instead of CC from comment #29)
> I was not able to uplift this bug to v1-train.  If this bug has dependencies
> which are not marked in this bug, please comment on this bug.  If this bug
> depends on patches that aren't approved for v1-train, we need to re-evaluate
> the approval.  Otherwise, if this is just a merge conflict, you might be
> able to resolve it with:
> 
>   git checkout v1-train
>   git cherry-pick -x -m1 f0ab15ca1a583080693b27be0d369d12e7dbd133
>   <RESOLVE MERGE CONFLICTS>
>   git commit

Hi Dominic can you look into what's causing this issue? Thanks!

	Dominic Kuo [:dkuo] Assignee
	Comment 31 • 12 years ago

Conflict resolved.

Uplifted commit f0ab15ca1a583080693b27be0d369d12e7dbd133 as:
v1-train: afd00e04a68b323eb09a6925000e740391b835f9

	Eric Pang [:epang] UX
	Comment 32 • 12 years ago

(In reply to Dominic Kuo [:dkuo] from comment #31)
> Conflict resolved.
> 
> Uplifted commit f0ab15ca1a583080693b27be0d369d12e7dbd133 as:
> v1-train: afd00e04a68b323eb09a6925000e740391b835f9

Thanks Dominic!