Closed Bug 1294358 Opened 8 years ago Closed 8 years ago

Add test for suspended videos still fire 'ended' event

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla51

Tracking Flags:

Tracking

Status

firefox51

---

fixed

People

(Reporter: u480271, Assigned: u480271)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Bug 1294358 - P1: Change test files to specify mime type. 8 years ago u480271 58 bytes, text/x-review-board-request	jya : review+	Details
Bug 1294358 - P2: Add test for 'ended' event firing for suspended video. 8 years ago u480271 58 bytes, text/x-review-board-request	jwwang : review+	Details
Bug 1294358 - P3: Added test video with no audio. 8 years ago u480271 58 bytes, text/x-review-board-request	jya : review+	Details
Bug 1294358 - P4: Shrink video size. 8 years ago u480271 58 bytes, text/x-review-board-request	jya : review+	Details

u480271

Assignee

Description

•

8 years ago

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.

u480271

Assignee

Updated

•

8 years ago

Assignee: nobody → dglastonbury

Blocks: 1276556

Status: NEW → ASSIGNED

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/70868/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70868/

Comment hidden (mozreview-request)

This exercises the decoder update logic path that doesn't rely on
AudioSink to advance time.

Review commit: https://reviewboard.mozilla.org/r/70870/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70870/

JW Wang [:jwwang] [:jw_wang]

Comment 3

•

8 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.

JW Wang [:jwwang] [:jw_wang]

Comment 4

•

8 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+

JW Wang [:jwwang] [:jw_wang]

Comment 5

•

8 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 on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70868/diff/1-2/

Comment hidden (mozreview-request)

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70870/diff/1-2/

Comment hidden (mozreview-request)

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70868/diff/2-3/

Comment hidden (mozreview-request)

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70870/diff/2-3/

u480271

Assignee

Comment 10

•

8 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.

Pulsebot

Comment 11

•

8 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

Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)

Comment 12

•

8 years ago

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

Flags: needinfo?(dglastonbury)

u480271

Assignee

Comment 13

•

8 years ago

(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)

Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)

Updated

•

8 years ago

Priority: -- → P3

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/73238/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73238/

Comment hidden (mozreview-request)

Make the test videos smaller so they are visible in the mochitest
iframe.

Review commit: https://reviewboard.mozilla.org/r/73240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73240/

Comment hidden (mozreview-request)

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70868/diff/3-4/

Comment hidden (mozreview-request)

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70870/diff/3-4/

Jean-Yves Avenard [:jya]

Comment 18

•

8 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+

Jean-Yves Avenard [:jya]

Comment 19

•

8 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+

u480271

Assignee

Comment 20

•

8 years ago

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+

u480271

Assignee

Updated

•

8 years ago

Attachment #8783429 - Flags: review+ → review?(jyavenard)

Jean-Yves Avenard [:jya]

Comment 21

•

8 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+

Pulsebot

Comment 22

•

8 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

Ryan VanderMeulen [:RyanVM]

Comment 23

•

8 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

Closed: 8 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.