1295923 - Fix the wrong behavior of loading mal-format VideoDocument not causing error

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Comment 1

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

When loading malformat video document, we should throw error when loading event dispatched.

Comment 2

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

You might also want to see below comment for this issue.
https://bugzilla.mozilla.org/show_bug.cgi?id=608634#c628

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Now we are able to fix this bug since bug 1329403 is fixed.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1295923 - remove the workaround since bug 1329403 is fixed and 'onload' won't be fired prematurely before the media element finishes loading.

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

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8828652 [details]
Bug 1295923 - remove the workaround since bug 1329403 is fixed and 'onload' won't be fired prematurely before the media element finishes loading.

https://reviewboard.mozilla.org/r/106000/#review106934

r+, after you have considered this:

::: dom/media/test/test_error_in_video_document.html:52
(Diff revision 1)
>  } else {
>    SimpleTest.waitForExplicitFinish();
>  
>    var f = document.createElement("iframe");
>    f.src = t.name;
> -  f.addEventListener("load", function() {
> +  f.addEventListener("load", check);

The previous version was:
`f.addEventListener("load", function() { SimpleTest.executeSoon(check); }, false);`
http://searchfox.org/mozilla-central/rev/41419149d7b84118d21747fd193f4133443be39b/dom/media/test/test_error_in_video_document.html#50
Changed in bug 1154802, reviewed by you! :-)

Are you sure about calling `check` directly now?

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8828652 [details]
Bug 1295923 - remove the workaround since bug 1329403 is fixed and 'onload' won't be fired prematurely before the media element finishes loading.

https://reviewboard.mozilla.org/r/106000/#review106934

> The previous version was:
> `f.addEventListener("load", function() { SimpleTest.executeSoon(check); }, false);`
> http://searchfox.org/mozilla-central/rev/41419149d7b84118d21747fd193f4133443be39b/dom/media/test/test_error_in_video_document.html#50
> Changed in bug 1154802, reviewed by you! :-)
> 
> Are you sure about calling `check` directly now?

https://hg.mozilla.org/mozilla-central/file/75ac238f8796/content/media/test/test_error_in_video_document.html
The earliest version that I can traced called setTimeout(check, 0);

I believe this is a workaround for 'onload' fires prematurely without bug 1329403 being fixed. So it should be fine now to call check() immmediately without any delay.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26db8aff32c8
remove the workaround since bug 1329403 is fixed and 'onload' won't be fired prematurely before the media element finishes loading. r=gerald

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/26db8aff32c8