Add test for suspended videos still fire 'ended' event

RESOLVED FIXED in Firefox 51

Status

()

P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kamidphish, Assigned: kamidphish)

Tracking

(Blocks: 1 bug)

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

There was a bug where suspending video decode causes videos not to fire ended event until they became visible. Add a test to ensure the desire behaviour still works.
Assignee: nobody → dglastonbury
Blocks: 1276556
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review68252

::: dom/media/test/test_background_video_suspend_ends.html:35
(Diff revision 1)
> +    // This test checks that 'ended' event is received for videos with
> +    // suspended video decoding. This is important for looping video logic
> +    // handling in HTMLMediaElement.
> +    waitUntilPlaying(v)
> +      .then(() => testVideoSuspendsWhenHidden(v))
> +      .then(() => waitUntilEnded(v))

Might be worth checking currentTime approximates the duration when ended.

Comment 4

2 years ago
mozreview-review
Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review68254
Attachment #8780031 - Flags: review?(jwwang) → review+

Comment 5

2 years ago
mozreview-review
Comment on attachment 8780032 [details]
Bug 1294358 - P2: Add test for 'ended' event firing for suspended video.

https://reviewboard.mozilla.org/r/70870/#review68256
Attachment #8780032 - Flags: review?(jwwang) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

2 years ago
mozreview-review-reply
Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review68252

> Might be worth checking currentTime approximates the duration when ended.

OK. Will do.

Comment 11

2 years ago
Pushed by dglastonbury@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/848141457f25
P1: Add test for 'ended' event firing for suspended video. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/886441076b5f
P2: Added test video with no audio. r=jwwang
(In reply to Wes Kocher (:KWierso) from comment #12)
> Backed out for xp media test failures:
> https://treeherder.mozilla.org/logviewer.html#?job_id=2057985&repo=autoland
> 
> https://hg.mozilla.org/integration/autoland/rev/cc222be2b97b

Thanks Wes, I'll take a look.
Flags: needinfo?(dglastonbury)
Priority: -- → P3
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 18

2 years ago
mozreview-review
Comment on attachment 8783430 [details]
Bug 1294358 - P4: Shrink video size.

https://reviewboard.mozilla.org/r/73240/#review70998

technically you aren't making them smaller, you just scaling them.
Attachment #8783430 - Flags: review?(jyavenard) → review+

Comment 19

2 years ago
mozreview-review
Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review71000

assuming you got them all this time :)
Attachment #8780031 - Flags: review?(jyavenard) → review+
Comment on attachment 8783429 [details]
Bug 1294358 - P3: Added test video with no audio.

r=me, carry r+ from jwwang.
Attachment #8783429 - Flags: review?(jwwang) → review+
Attachment #8783429 - Flags: review+ → review?(jyavenard)

Comment 21

2 years ago
mozreview-review
Comment on attachment 8783429 [details]
Bug 1294358 - P3: Added test video with no audio.

https://reviewboard.mozilla.org/r/73238/#review71252
Attachment #8783429 - Flags: review?(jyavenard) → review+

Comment 22

2 years ago
Pushed by dglastonbury@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/15de17e6f1e1
P1: Change test files to specify mime type. r=jwwang,jya
https://hg.mozilla.org/integration/autoland/rev/24f0a2f224c4
P2: Add test for 'ended' event firing for suspended video. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/25b8cda49700
P3: Added test video with no audio. r=jya
https://hg.mozilla.org/integration/autoland/rev/65cc7b37cf3f
P4: Shrink video size. r=jya

Comment 23

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/15de17e6f1e1
https://hg.mozilla.org/mozilla-central/rev/24f0a2f224c4
https://hg.mozilla.org/mozilla-central/rev/25b8cda49700
https://hg.mozilla.org/mozilla-central/rev/65cc7b37cf3f
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.