Closed
Bug 462570
Opened 16 years ago
Closed 16 years ago
Network state and Ready state names in HTMLMediaElement are not as defined in spec
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
VERIFIED
FIXED
People
(Reporter: cajbir, Assigned: cpearce)
References
()
Details
(Keywords: verified1.9.1)
Attachments
(1 file, 4 obsolete files)
48.39 KB,
patch
|
cpearce
:
review+
cpearce
:
superreview+
|
Details | Diff | Splinter Review |
Section 4.8.10 of the spec outlines the ready state and network states in HTMLMediaElement. These have recently changed and the current implementation we have in nsIDOMHTMLMediaElement is now incorrect.
Assignee | ||
Comment 1•16 years ago
|
||
Renames network state and ready state. I updated unit tests to pass. I recreated content/media/video/test/test_constants.html based on http://simon.html5.org/test/html/dom/interfaces/HTMLElement/HTMLMediaElement/const-unsigned-short/001.htm and I kept the HTMLMediaElement.prototype.NETWORK_* tests, but left them as todo, since they don't pass. I think HTMLMediaElement.prototype.NETWORK_* should work, but currently it doesn't.
Assignee: nobody → chris
Attachment #346559 -
Flags: superreview?(roc)
Attachment #346559 -
Flags: review?(chris.double)
Comment on attachment 346559 [details] [diff] [review] Patch v1 nice
Attachment #346559 -
Flags: superreview?(roc) → superreview+
Flags: blocking1.9.1+
Assignee | ||
Comment 3•16 years ago
|
||
Comment on attachment 346559 [details] [diff] [review] Patch v1 Patch is bitrotten.
Attachment #346559 -
Attachment is obsolete: true
Attachment #346559 -
Flags: review?(chris.double)
Assignee | ||
Comment 4•16 years ago
|
||
Updated to account for bitrot and changes in the spec. I removed some events which are no longer in the spec as well, and added the loadeddata event.
Attachment #352660 -
Flags: superreview?(roc)
Attachment #352660 -
Flags: review?(chris.double)
Reporter | ||
Updated•16 years ago
|
Attachment #352660 -
Flags: review?(chris.double) → review+
Reporter | ||
Comment 5•16 years ago
|
||
Comment on attachment 352660 [details] [diff] [review] Patch v2 > +GK_ATOM(loadedfirstframe, "loadeddata") Copy/Paste error? r+ with that fixed
Assignee | ||
Comment 6•16 years ago
|
||
As Patch v2 with review comment addressed. r+ chris.double.
Attachment #352660 -
Attachment is obsolete: true
Attachment #352663 -
Flags: superreview?(roc)
Attachment #352663 -
Flags: review+
Attachment #352660 -
Flags: superreview?(roc)
Attachment #352663 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Whiteboard: [needs landing]
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Whiteboard: [needs landing]
Assignee | ||
Comment 7•16 years ago
|
||
As v2.1 but unbitrotten. r+ chris.double sr+ roc.
Attachment #352663 -
Attachment is obsolete: true
Attachment #352954 -
Flags: superreview+
Attachment #352954 -
Flags: review+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Whiteboard: [needs landing]
Assignee | ||
Comment 8•16 years ago
|
||
Now with even less bitrot!
Attachment #352954 -
Attachment is obsolete: true
Attachment #352965 -
Flags: superreview+
Attachment #352965 -
Flags: review+
Reporter | ||
Comment 9•16 years ago
|
||
Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/3a33323d7270
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs landing][baking for 1.9.1]
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 10•15 years ago
|
||
When this lands on 1.9.1, the mochitest for bug 461680 needs to be changed to listen for 'loadeddata' event, rather than 'loadedmetadata'.
Comment 11•15 years ago
|
||
Comment on attachment 352965 [details] [diff] [review] Patch v2.3 unbitrotten++ [Checkin: Comment 9] (In reply to comment #10) > When this lands on 1.9.1, the mochitest for bug 461680 needs to be changed to This patch does not apply (cleanly) to 1.9.1 branch. > listen for 'loadeddata' event, rather than 'loadedmetadata'. This has not been done (yet) on trunk...
Attachment #352965 -
Attachment description: Patch v2.3 unbitrotten++ → Patch v2.3 unbitrotten++
[Checkin: Comment 9]
Updated•15 years ago
|
Flags: in-testsuite+
Keywords: checkin-needed
Comment 12•15 years ago
|
||
Comment on attachment 352965 [details] [diff] [review] Patch v2.3 unbitrotten++ [Checkin: Comment 9] (In reply to comment #11) > > listen for 'loadeddata' event, rather than 'loadedmetadata'. > > This has not been done (yet) on trunk... My bad: it has :-> Could you include this change in the 1.9.1 patch ?
Comment 13•15 years ago
|
||
(In reply to comment #10) > When this lands on 1.9.1, the mochitest for bug 461680 needs to be changed to > listen for 'loadeddata' event, rather than 'loadedmetadata'. It should be fine without it, it can be changed separately or just ignored.
Assignee | ||
Comment 14•15 years ago
|
||
*sigh*... This requires bug 467972 to land on 1.9.1 before this can land on 1.9.1. I'll unbitrot the patch once bug 467972 has landed...
Assignee | ||
Comment 15•15 years ago
|
||
We need to land the patch for bug 469598 to land on 1.9.1 before the patch for this bug can land on 1.9.1.
Depends on: 469598
Assignee | ||
Comment 16•15 years ago
|
||
We need to land the patch for bug 466432 to land on 1.9.1 before the patch for this bug can land on 1.9.1.
Depends on: 466432
Pushed bc83afa92f67 to 1.9.1
Keywords: fixed1.9.1
Whiteboard: [needs landing][baking for 1.9.1]
Comment 18•15 years ago
|
||
Seeing as there hasn't been any discussions about this bug for 3 months and it's been in mochitest, I'm assuming there aren't any residual issues. I'm moving this to verified as a result. If anyone has any qualms, feel free to bring them up.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•