[Video] pause the video and back to beginning when a video plays to end



Firefox OS
4 years ago
3 years ago


(Reporter: johnhu, Assigned: johnhu)


Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

When playing a very short video, like 2sec, we don't have ht opportunity to see detail info, delete, or share the video in the player view. The current behavior is to go back to list view when it plays to end.

As per rob's comment, we discussed offline, he thought we should pause the video and go back to the beginning of video when the video plays to end.
Assignee: nobody → johu
Created attachment 811807 [details]
pause the video and moves to the begins

When video plays to end, the player pauses the video playing and moves the beginning of this video.
Attachment #811807 - Flags: review?(dflanagan)
Created attachment 811813 [details]

Hi Rob,

Please also review this one. I followed your design to pause the video and move the video to the beginning when it plays to end.
Attachment #811813 - Flags: ui-review?(rmacdonald)
Comment on attachment 811807 [details]
pause the video and moves to the begins

The change you made looks fine.

In the process of checking that hidePlayer() is still called correctly to update the metadata, however, I discovered a related bug that we should fix, either here, or in a new bug.

hidePlayer() takes a boolean argument named updateMetadata, but also has a nested function named updateMetadata.  This means that when you call hidePlayer() with the argument false, that argument is being ignored.  That should be an easy bug to fix.

Also note another implication of this change. By pausing the video and not returning to the list of thumbnails, video metadata parsing does not automaticaly resume.  If new videos are being scanned, the user will have to manually return to the thumbnail list before more thumbanails are added to the list.  If this is a problem, then a deeper refactoring will be required here.
Attachment #811807 - Flags: review?(dflanagan) → review+

Note that the travis build failed on your patch. It appears unrelated, but it could be that other problems prevented tests from running. If there are any video app tests that rely on the behavior you are changing here, don't forget to update them, too.
I rename the "updateMetadata" argument as updateVideoMetadata to prevent the conflict between them.

I also found the travis failed. That's not related to video app. Thanks for this.
Blocks: 903920
Attachment #811813 - Attachment is obsolete: true
Attachment #811813 - Flags: ui-review?(rmacdonald)
merged to master:
blocking-b2g: --- → 1.3?
Last Resolved: 4 years ago
Resolution: --- → FIXED
Nominate it as 1.3? because bug 903920 depends on this one.
bug 903920 is no more 1.3+, clear the 1.3? flag.
blocking-b2g: 1.3? → ---
You need to log in before you can comment on or make changes to this bug.