466410 - videoWidth and videoHeight should be valid when loadedmetadata fires

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P2)

Description

[Matthew Gregan [:kinetik]]

Per section 4.8.10.5.11.4, where it talks about firing the loadedmetadata, "Once enough of the media data has been fetched to determine the duration of the media resource, its dimensions, and other metadata".

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: mochitest

This happens because nsOggDecoder updates the videoWidth and videoHeight during invalidate, which may not have happened by the time loadedmetadata fires.  In some cases, it's possible for invalidate never to occur, e.g. if the video is created outside of the document and never attached or if the element is set to display: none.

Comment 2

[Matthew Gregan [:kinetik]]

Attachment: mochitest

Test display: none case too.

Comment 3

[Matthew Gregan [:kinetik]]

I posted a message to the WhatWG spec list requesting clarification on this: http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2008-November/017412.html

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This makes the size valid when metadataloaded fires.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed http://hg.mozilla.org/mozilla-central/rev/e6566d187edd

Comment 6

[Daniel Holbert [:dholbert]]

This seems to have caused reftest failures in ogg-video/basic-1.html' and 'ogg-video/zoomed-1.html' on the linux unittest box:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1232599526.1232605622.2267.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1232600679.1232605576.2202.gz

Backed out to fix bustage:
http://hg.mozilla.org/mozilla-central/rev/c0d1d49217cc

Comment 7

[Daniel Holbert [:dholbert]]

The same tests failed on mac:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1232599526.1232603701.31322.gz

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: updated patch

The frame was not being reflowed after the video intrinsic size became known, because we were just calling UpdateMediaSize and then setting mSizeChanged to false, but we need to do what Invalidate does and also mark the frame as needing reflow. So in this version of the patch I just call Invalidate to do all those things.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed http://hg.mozilla.org/mozilla-central/rev/4bc1bd14f5a0

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/910ac31768ab