[music] implement songs picker to support "pick" activity

RESOLVED FIXED in 1.1 CS (11may)

Status

RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: dkuo, Assigned: dkuo)

Tracking

unspecified
1.1 CS (11may)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:leo+, b2g18 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
For v1.1 features, some apps like email and sms/mms need to pick songs to complete the user stories.

email: bug 838007
mms: bug 840044

so we need songs picker to support both email and mms to attach audio files.
(Assignee)

Updated

6 years ago
Blocks: 838007, 840044
Duplicate of bug 836497?
(Assignee)

Comment 2

6 years ago
Bug 836497 is for email user story only, and this one is for music app to support general pick activity for every app that needs pick, like sms/mms, and maybe more in the future.
Nominating as leo+, as this is blocking a MMS P1 US bug 840044.
Adding [NO-UPLIFT] whiteboard, as it can not be uplifted until the complete MMS functionality is ready.
blocking-b2g: --- → leo?
Whiteboard: [NO-UPLIFT]
blocking-b2g: leo? → leo+
remove no_uplift as this is does not have a major dependency with MMS
Whiteboard: [NO-UPLIFT]
(Assignee)

Updated

6 years ago
Depends on: 831265
(Assignee)

Comment 5

6 years ago
Created attachment 735670 [details]
implement song picker

Steve,

To speed up the developing of MMS features, I have implemented a first version of song picker for you to test when you are working on MMS, this patch should be enough for you to get the file/blob from the song picker via pick activity, let me know if there have any problems, thanks.
Attachment #735670 - Flags: feedback?(schung)
This patch works well. CC other mms developers. Thanks

Updated

6 years ago
Attachment #735670 - Flags: feedback?(schung) → feedback+
how to move this forward ?
(Assignee)

Comment 8

6 years ago
I haven't get a chance to finish this patch and there are still some works to do. The major problem is it take few seconds to display all the songs in music picker, I am trying to make it faster.
Can you land first, and file a followup bug for perf improvement on it?
(Assignee)

Comment 10

6 years ago
Okay, the sounds good to me, I will cleanup that patch and file a perf bug as a followup.
Marcia, this will be landing soon.  can you add a 1.1 test for this in moztrap?  (i dont know where the spec is for this).  thanks!
Flags: in-moztrap?(mozillamarcia.knous)
Dominic can you please make this a priority to land the final patch today?
(Assignee)

Comment 13

6 years ago
Comment on attachment 735670 [details]
implement song picker

David,

I have cleaned up this patch and tried to make it looks better as it is, we need this land to support picking files from MMS since gallery and video were already enabled. You may found that the features of MMS are not completed yet so I have also modified email for you to test music picker, after this patch get a r+, I will remove it before we land it.
Attachment #735670 - Attachment description: WIP - implement song picker → implement song picker
Attachment #735670 - Flags: review?(dflanagan)
(Assignee)

Comment 14

6 years ago
(In reply to Dietrich Ayala (:dietrich) from comment #12)
> Dominic can you please make this a priority to land the final patch today?

Just finished cleanup my patch and hope it will not too buggy...

Updated

6 years ago
Target Milestone: --- → 1.1 CS (11may)
Comment on attachment 735670 [details]
implement song picker

Dominic,

I'm giving a tentative r+ because the MMS team really want this landed, even without the perf improvements.

See my comments on github. I think you should at least change the enumerateAll to enumerate() and avoid enumerating the TilesView at the same time as enumerating all song titles.

If you fix those things, go ahead and land, but then please file a followup bug to continue to work on the performance issues.
Attachment #735670 - Flags: review?(dflanagan) → review+
(Assignee)

Comment 16

6 years ago
David,

Thanks for the review, I have revised my patch as your suggestions on github, I will file a new bug to record the performance issue, and I am landing it.
(Assignee)

Comment 17

6 years ago
Landed on master: 4617db5dcc927c2fe0e05769db42bab18b59efb5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Blocks: 868190

Updated

6 years ago
Blocks: 868225
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 4617db5dcc927c2fe0e05769db42bab18b59efb5
  <RESOLVE MERGE CONFLICTS>
  git commit
Dominic, could you please try to uplift yourself ?
Flags: needinfo?(dkuo)
(Assignee)

Comment 20

6 years ago
This bug needs some changes from the patch of bug 831265(The dependency is already set), which are the conflicts that John has encountered, I have re-request the approval-gaia-v1 flag for bug 831265, if we cannot get approval, I will uplift this myself, thanks.
Flags: needinfo?(dkuo)
status-b2g18: --- → affected

Updated

6 years ago
Duplicate of this bug: 868945
Whiteboard: [NO_UPLIFT]
(Assignee)

Comment 22

5 years ago
Conflict resolved.

Uplifted commit 4617db5dcc927c2fe0e05769db42bab18b59efb5 as:
v1-train: fcaa90923894211c19b3544b5cb16eb0db5a6286
status-b2g18: affected → fixed
Whiteboard: [NO_UPLIFT]

Comment 23

5 years ago
Added Music Suite Test Case #8910 - Test that a Music picker activity functions for creating MMS messages.
Flags: in-moztrap?(mozillamarcia.knous) → in-moztrap+

Updated

4 years ago
status-b2g-v1.3: --- → affected
status-b2g-v1.4: --- → affected
status-b2g-v2.0: --- → affected
Whiteboard: qawanted, [2.0-flame-test-run-1]
The flags here are entirely incorrect. Please contact your QA lead to figure out how bugs should be filed in test runs.
status-b2g-v1.3: affected → ---
status-b2g-v1.4: affected → ---
status-b2g-v2.0: affected → ---
Flags: needinfo?(bmorgan)
Whiteboard: qawanted, [2.0-flame-test-run-1]
Switching needinfo to Kevin since I am not getting a response from bmorgan.
Flags: needinfo?(bmorgan) → needinfo?(ktucker)
I spoke to bmorgan and the lead over in SLC to go over the proper procedure once again to ensure this doesn't happen again. They will also be receiving even more training this week.
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.