889164 - [Video Player] Slider position is not updated when controls are hide and Player ended

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title : Slider position is not updated when controls are not visible and Player ended
2. Precondition : Video player app should be working and mobile is connected to internet
3. Tester's Action : 1. Open youtube.com
                     2. Play any video
                     3. While playing touch the screen so that controls will get visible -> notice the slider position.
                     4. touch the screen again to hide the controls -> Play the video till the end of the file
                     5. Once player is ended touch the screen to see slider position.
 
4. Detailed Symptom (ENG.) : Now it shows the slider position that was seen in step 3.

5. Expected : Slider should be updated properly when player is ended.

6.Reproducibility: Y
1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced
8.Gaia Revision: 65e861f5941cab66815a2f79b0bc0a6a0fc8f6fe
9.Personal email id: [e-mail]

Comment 1

[Leo]

Attachment: Pointer to pull request

Hi David,

Please review the patch

Comment 2

[David Flanagan [:djf]]

Comment on attachment 769987 [details]
Pointer to pull request

r+ for breaking out updateSlider() into a separate function.
But r- for calling updateSlider() from playerEnded().

I think it would be better to call updateSlider from setControlsVisibility.  Currently there is code at line 155 that updates the time but not the slider. I think you should replace that line with a call to updateSlider(). If you do that, I don't think you'll need the call in playerEnded.  (But if I'm wrong, then it is safe to leave it in player ended, of course.)

I'll be on vacation, so you'll want to ask dkuo or daleharvey for followup review.

Comment 3

[Leo]

Attachment: Pointer to pull request

Hi Dale,

I have made changes as per David's comments. Please review the patch.

Comment 4

[Dale Harvey (:daleharvey)]

Comment on attachment 770706 [details]
Pointer to pull request

Nice patch, cheers

Comment 5

[Dale Harvey (:daleharvey)]

Merged in: https://github.com/mozilla-b2g/gaia/commit/9bbedef67226c1185d67b9e1b7332691bea665b0

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

John - is this coming up in your uplift queries?

Comment 7

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

It isn't in the queries that I have from B2G_Landing as the bug is still NEW.  Can it be RESOLVED->FIXED?

Comment 8

[bhavana bajaj [:bajaj]]

Dale, David : can this be resolved fixed for it to be uplifted given comment# 7?

Comment 9

[David Flanagan [:djf]]

Yep, it looks like this was just a mistake.

Comment 10

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

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 9bbedef67226c1185d67b9e1b7332691bea665b0
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 11

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

This bug was partially uplifted.

Uplifted 9bbedef67226c1185d67b9e1b7332691bea665b0 to:
v1.2 already had this commit

Commit 9bbedef67226c1185d67b9e1b7332691bea665b0 didn't uplift to branch v1-train

Comment 12

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

This bug was partially uplifted.

Uplifted 9bbedef67226c1185d67b9e1b7332691bea665b0 to:
v1.3 already had this commit

Commit 9bbedef67226c1185d67b9e1b7332691bea665b0 didn't uplift to branch v1-train

Comment 13

[Leo]

This issue was merged in V1.1 manually.

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #10)
> 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 9bbedef67226c1185d67b9e1b7332691bea665b0
>   <RESOLVE MERGE CONFLICTS>
>   git commit