Closed Bug 1175523 Opened 9 years ago Closed 9 years ago

Unprefix mozSrcObject (to srcObject)

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox41 --- affected
firefox42 --- fixed

People

(Reporter: jib, Assigned: jib)

References

(
URL
)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files)

MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject
40 bytes, text/x-review-board-request
jib
: review+
jib
: review+
Details
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject
40 bytes, text/x-review-board-request
jib
: review+
Details 
HTMLMediaElement.srcObject is in the html 5.1 spec now (see URL), so it seems time.
Assignee: nobody → jib
Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject
Attachment #8633306 - Flags: review?(roc)
Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject
Attachment #8633307 - Flags: review?(pehrsons)
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

https://reviewboard.mozilla.org/r/13199/#review11799

Ship It!
Attachment #8633306 - Flags: review?(roc) → review+
Attachment #8633307 - Flags: review?(pehrsons) → review+
Comment on attachment 8633307 [details]
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject

https://reviewboard.mozilla.org/r/13201/#review11797

Looks good, but at least the double `SimpleTest.finish()` needs to get fixed before landing.

::: dom/media/tests/mochitest/test_getUserMedia_constraints.html:98
(Diff revision 1)
> -     "Unexpected support (please update test): " + unexpected);
> +     "Unanticipated support (please update test): " + unexpected);

This change doesn't seem to belong in this patch.

::: dom/media/tests/mochitest/test_peerConnection_callbacks.html:1
(Diff revision 1)
> -<!DOCTYPE HTML>
> +<!DOCTYPE HTML>

Same with this.

::: dom/media/test/test_streams_srcObject.html:44
(Diff revision 1)
> +      SimpleTest.finish();

There appears to be two `SimpleTest.finish()` now. Are they racing?
https://reviewboard.mozilla.org/r/13201/#review11797

> Same with this.

whoops, I trampled on a byte order mark. Restored.

> This change doesn't seem to belong in this patch.

I was trying to sneak it in. Turns out using the word 'unexpected' in an is() test, not the best idea (no harm other than irritation while searching for real test failures). I'll open a separate bug.
Comment on attachment 8633307 [details]
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject

Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject
Attachment #8633307 - Flags: review+ → review?(pehrsons)
Comment on attachment 8633307 [details]
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject

https://reviewboard.mozilla.org/r/13201/#review11929

Ship It!
Attachment #8633307 - Flags: review?(pehrsons) → review+
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject
Attachment #8633306 - Flags: review+ → review?(roc)
Comment on attachment 8633307 [details]
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject

Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject
Attachment #8633307 - Flags: review+ → review?(pehrsons)
Attachment #8633306 - Flags: review?(roc) → review+
Attachment #8633307 - Flags: review?(pehrsons) → review+
Rebased for good measure. I find it hard to tell in the hg bookmarks workflow whether rebasing is needed for landing. Anyone have a good solution to this?
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

https://reviewboard.mozilla.org/r/13199/#review11951

Ship It!
Attachment #8633306 - Flags: review+
webidl changes need DOM peer review. Also, Try link? :)
Keywords: checkin-needed
Thanks for catching that!

Try links are hidden in the Review Board UX now (3rd collapsed "Review request changed" from the top).
or https://treeherder.mozilla.org/#/jobs?repo=try&revision=15bbd3d14a60
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject
Attachment #8633306 - Flags: review?(roc)
Attachment #8633306 - Flags: review?(bugs)
Attachment #8633306 - Flags: review+
Attachment #8633306 - Flags: review?(roc) → review+
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

Nit, we try to implement WhatWG HTML spec, not W3C HTML 5.x.

HTML spec has
attribute MediaProvider? srcObject;
but the patch 
attribute MediaStream? srcObject;

Do we have plans to support MediaProvider there?
I think we need intent-to-implement/ship on dev.platform especially in this particular case since we're not following the spec here.
http://logs.glob.uno/?c=content#c308536
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

and clearing the review request until that is done.

Note, the worry is that we implement something which throws in cases the spec says it shouldn't throw. (when using the setter with a type we don't support yet)
Attachment #8633306 - Flags: review?(bugs)
Attachment #8633306 - Flags: review?(roc)
Attachment #8633306 - Flags: review?(bugs)
Attachment #8633306 - Flags: review+
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject
Attachment #8633307 - Flags: review+ → review?(pehrsons)
Comment on attachment 8633307 [details]
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject

Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject
Comment on attachment 8633306 [details]
MozReview Request: Bug 1175523 - add HTMLMediaElement.srcObject alias to .mozSrcObject

Just a rebase.
Attachment #8633306 - Flags: review?(roc)
Attachment #8633306 - Flags: review?(bugs)
Attachment #8633306 - Flags: review+
Attachment #8633307 - Flags: review?(pehrsons) → review+
It's been two weeks, with few comments, all positive. OK to land?

Would love to get this in for 42 (it's a safe alias for the prefixed version which remains for now).
Flags: needinfo?(bugs)
Comment on attachment 8633307 [details]
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject

Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject
Attachment #8633307 - Flags: review+ → review?(pehrsons)
Comment on attachment 8633307 [details]
MozReview Request: Bug 1175523 - update most (but not all) tests to use elem.srcObject over .mozSrcObject

Fixed merge accident.
Attachment #8633307 - Flags: review?(pehrsons) → review+
From #content today: 12:56:54 -  @smaug:	 yeah, I think it should be fine
Flags: needinfo?(bugs)
https://hg.mozilla.org/mozilla-central/rev/dff0fcf398a1
https://hg.mozilla.org/mozilla-central/rev/79d3b8d91250
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: