850574 - [Gaia][Video] Support sorting file by date

Status: RESOLVED FIXED

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

Description

[[e-mail], [e-mail]]

Videos should be sorting by date in list view.

Repro steps:
1. Launch Video app
2. Create video by Bluetooth file transfer/Email/other file system
3. The created video will be added in bottom of the list view.

Expect:
The created video should be added and sorting by date.

Actual result:
The created video will be added in bottom of the list view.

Comment 1

[[e-mail], [e-mail]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8612

Pointer to Github pull-request

Comment 2

[[e-mail], [e-mail]]

Comment on attachment 724332 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8612

Note: 
David has reviewed the previous patch of the pr with some comment(https://github.com/mozilla-b2g/gaia/pull/8517).
If other reviewers want to help on this, you could reference it too.

Comment 3

[[e-mail], [e-mail]]

Since Bug 838655 and Bug 838027 will be supported in v1.1, we have more scenarios for creating video. Sorting video files by date is more meaningful for user experience. So I think we better to support the feature, nominating.

Comment 4

[David Flanagan [:djf]]

Comment on attachment 724332 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8612

The code looks good to me. This is a long-standing bug, and it is good to have it fixed. I agree that this would be good to land on leo.

I'm giving r+. Before merging, however, please ensure that you have tested the case where a new video is inserted and is not the newest one. I think you'll have to start the video app, then do a USB mass storage session and copy a file to the phone. Then, when you're done with the USB transfer, make sure the app puts the video in the right position in the list.  Another way to test it would be to copy a video file to the phone but give it a name wiht '.' as the first character. MediaDB will ignore the file with that prefix.  Then record a newer video and view it in the app.  Then go back and rename the file to remove the '.' prefix.  Now ensure that when the video app scans again, the file goes in the right spot.

Also, I notice that the list of videos is not showing the duration of the video in the list of videos. Please check whether this is a regression caused by this bug. If it is a regression, fix it. And if not, please file a new bug about the issue.

Note also that I've just landed Alive's patch to take out mozRequestFullScreen(), so your code may need some manual merging.

Comment 5

[[e-mail], [e-mail]]

(In reply to David Flanagan [:djf] from comment #4)
> Comment on attachment 724332 [details]
> Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8612
> 
> The code looks good to me. This is a long-standing bug, and it is good to
> have it fixed. I agree that this would be good to land on leo.
> 
> I'm giving r+. Before merging, however, please ensure that you have tested
> the case where a new video is inserted and is not the newest one. I think
> you'll have to start the video app, then do a USB mass storage session and
> copy a file to the phone. Then, when you're done with the USB transfer, make
> sure the app puts the video in the right position in the list.  Another way
Thanks for your reviewing effort again. I have verified the functionality according above steps after refined the patch. It works fine for me.  

> Also, I notice that the list of videos is not showing the duration of the
> video in the list of videos. Please check whether this is a regression
> caused by this bug. If it is a regression, fix it. And if not, please file a
> new bug about the issue.
It's a nit. I should do textTruncate() after added the thumbnail in the thumbnail list. Fixed.

> Note also that I've just landed Alive's patch to take out
> mozRequestFullScreen(), so your code may need some manual merging.
Merged. Thanks for your reminder.:)

Comment 6

[[e-mail], [e-mail]]

https://github.com/mozilla-b2g/gaia/pull/8612
Since the pr is merged, we can close the issue now.
We're having a sorted video list in Video app.:)
Thanks.

Comment 7

[Alex Keybl [:akeybl]]

Jaime - is this desired behavior?

Comment 8

[David Flanagan [:djf]]

Alex: yes this is desired.  The app sorts videos by date, newest first. The bug was that if new videos were added while the video app was running, it would put them at the end of the list instead of at the start of the list.  This patch fixes that. This is not a UX change.

Comment 9

[Al Tsai [:atsai]]

renom it. It should be leo+ becuase it's a v1.1 feature blocker.

Comment 10

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted commit ac032d2082f0f9fe75b005ec7e5e9a154362b8bd as:
v1-train: cefdf2681c8f9c2085ccbaa43e3a7709079c8d84