Closed Bug 1295923 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: ctai, Assigned: jwwang)

References

Details

Attachments

(1 file)

No description provided.
When loading malformat video document, we should throw error when loading event dispatched.
You might also want to see below comment for this issue.
https://bugzilla.mozilla.org/show_bug.cgi?id=608634#c628
Now we are able to fix this bug since bug 1329403 is fixed.
Assignee: nobody → jwwang
Depends on: CVE-2017-5396
Attachment #8828652 - Flags: review?(gsquelart)
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?
Attachment #8828652 - Flags: review?(gsquelart) → review+
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.
Pushed by jwwang@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/26db8aff32c8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.