Update MediaRecorder to spec by firing MediaRecorderErrorEvent for errors

RESOLVED FIXED in Firefox 57

Status

()

defect
P2
normal
Rank:
21
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: robert.swain, Assigned: bryce)

Tracking

(Blocks 1 bug, {dev-doc-complete})

48 Branch
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(9 attachments)

2.89 KB, text/html
Details
59 bytes, text/x-review-board-request
jib
: review+
smaug
: review+
Details
59 bytes, text/x-review-board-request
jib
: review+
Details
59 bytes, text/x-review-board-request
jib
: review+
smaug
: review+
Details
59 bytes, text/x-review-board-request
jib
: review+
Details
59 bytes, text/x-review-board-request
jib
: review+
Details
59 bytes, text/x-review-board-request
jib
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
jib
: review+
Details
(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.102 Safari/537.36

Steps to reproduce:

I am testing some MediaRecorder functionality on FireFox Dev Edition 48.0a2 (2016-05-26) on OS X 10.11.5 using this https://jsfiddle.net/rshodaw8/9/

Run that fiddle and it will try to get one default resolution video stream and one 160x90 video stream. It then uses MediaRecorder to record the first stream for 5 seconds and then it swaps out the first video track with the second.

NOTE: Whether this operation should work or not is a question for another ticket and I will raise that elsewhere.


Actual results:

I see the following output in the console:

Use of getAttributeNode() is deprecated. Use getAttribute() instead._dist-editor.js:2:25246
Starting recorder
Recording...
Waiting for 5 seconds...
done
Replacing track with low resolution
Waiting for 5 seconds...
ERROR:  error { target: MediaRecorder, isTrusted: true, name: "GenericError", currentTarget: MediaRecorder, eventPhase: 2, bubbles: false, cancelable: false, defaultPrevented: false, timeStamp: 1464257988236837, originalTarget: MediaRecorder, explicitOriginalTarget: MediaRecorder }
Recording stopped
done
Stopping recorder
ERROR: InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable

So, sometime after swapping the tracks but during the second 5s timeout, an error is raised. This error is not instanceof Error. It has no message property. It doesn't seem to adhere to the spec, unless I am mistaken.


Expected results:

Either there should be no error and the track switching would be supported, or more likely right now the IllegalStreamModification error should have been raised: https://developer.mozilla.org/en-US/docs/Web/API/MediaRecorder/onerror
(Reporter)

Updated

3 years ago
Component: Untriaged → Audio/Video: Recording
OS: Unspecified → Mac OS X
Product: Firefox → Core
Hardware: Unspecified → x86_64
Our current implementation is [1]. I'll also note that the spec has changed here and we're not up to date with it yet.


[1] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/RecordErrorEvent.webidl
Blocks: 1224079
Status: UNCONFIRMED → NEW
Rank: 35
Ever confirmed: true
Priority: -- → P2
Rank: 35 → 25
(Reporter)

Comment 2

3 years ago
Posted file track-switch.html
This is an HTML/JS file to ease debugging. It does the same as the fiddle which is apparently difficult to test.
Firefox currently fires a non-spec event with a name property. Used like this:

    recorder.onerror = e => { throw new Error(e.name); }

Instead, we need to update to the latest spec and implement MediaRecorderErrorEvent [1]:

    recorder.onerror = e => { throw e.error };

Part of the implementation work will be to capture the callstack of recorder.start(), to include as stack in the e.error DOMException object, like the spec encourages [2].

[1] https://w3c.github.io/mediacapture-record/MediaRecorder.html#idl-def-ErrorEvent
[2] https://w3c.github.io/mediacapture-record/MediaRecorder.html#erroreventinit
Rank: 25 → 21
Summary: MediaRecorder error callback called with object that is not instanceof Error → Update MediaRecorder to spec by firing MediaRecorderErrorEvent for errors
OS: Mac OS X → All
Hardware: x86_64 → All
Workaround:

    recorder.onerror = e => { throw e.error || new Error(e.name); };
(Assignee)

Updated

2 years ago
Assignee: nobody → bvandyk
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 13

2 years ago
I've got some further work in progress around capturing the stack trace for the DOMException. However I've got some leaks in there that I'm yet to sort out. As such I'm opting for review of the existing, (hopefully) working code, and will follow up with capturing the stack traces.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 21

2 years ago
mozreview-review
Comment on attachment 8894284 [details]
Bug 1275856 - Add MediaRecorderErrorEvent webidl files and update moz.build.

https://reviewboard.mozilla.org/r/165368/#review170856

Lgtm with duplicate entry removed. Note you'll need a DOM reviewer as well for touching webidl.

::: dom/webidl/moz.build:684
(Diff revision 2)
>      'MediaKeyStatusMap.webidl',
>      'MediaKeySystemAccess.webidl',
>      'MediaList.webidl',
>      'MediaQueryList.webidl',
>      'MediaRecorder.webidl',
> +    'MediaRecorderErrorEvent.webidl',

The pattern here appears to be: put it either under WEBIDL_FILES or GENERATED_EVENTS_WEBIDL_FILES, but not both. I suspect we want the latter?
Attachment #8894284 - Flags: review?(jib) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 26

2 years ago
mozreview-review
Comment on attachment 8894285 [details]
Bug 1275856 - Fire MediaRecorderErrorEvent in media recorder.

https://reviewboard.mozilla.org/r/165370/#review171014

Need to fire only valid errors, populate message field, and add stack.

Also, more work (in new or same issue): A comparison to the spec reveals that firing an event is supposed to be terminal, and "set state to inactive" [1]. For SecurityError we should also "discard any data that it has gathered".

::: dom/media/MediaRecorder.cpp:1401
(Diff revision 4)
> +    case NS_ERROR_DOM_INVALID_STATE_ERR:
> +    case NS_ERROR_DOM_NOT_SUPPORTED_ERR:

You've added two new errors here. Why?

According to the spec [1], the only two errors that can be fired are "SecurityError", and "UnknownError" for everything else.

This means any gecko-specifc debug info about the actual failure (like the aRv number) would need to be captured in the error.message field instead.

[1] https://w3c.github.io/mediacapture-record/MediaRecorder.html#dom-mediarecorder-start

::: dom/media/MediaRecorder.cpp:1406
(Diff revision 4)
> -  default:
> +    default:
> -    errorMsg = NS_LITERAL_STRING("GenericError");
> +      init.mError = DOMException::Create(NS_ERROR_DOM_ABORT_ERR);
>    }

Missing the stack part.

::: dom/media/MediaRecorder.cpp:1407
(Diff revision 4)
> -    break;
> -  case NS_ERROR_OUT_OF_MEMORY:
> -    errorMsg = NS_LITERAL_STRING("OutOfMemoryError");
> -    break;
> +      break;
> -  default:
> +    default:
> -    errorMsg = NS_LITERAL_STRING("GenericError");
> +      init.mError = DOMException::Create(NS_ERROR_DOM_ABORT_ERR);

NS_ERROR_DOM_UNKNOWN_ERR
Attachment #8894285 - Flags: review?(jib) → review-

Comment 27

2 years ago
mozreview-review
Comment on attachment 8894286 [details]
Bug 1275856 - Remove now unused RecrodErrorEvent.

https://reviewboard.mozilla.org/r/165372/#review171018
Attachment #8894286 - Flags: review?(jib) → review+

Comment 28

2 years ago
mozreview-review
Comment on attachment 8894287 [details]
Bug 1275856- Update tests to reflect new MediaRecorderErrorEvent.

https://reviewboard.mozilla.org/r/165374/#review171020

Some nits and some more changes needed if we end up fixing the state change in this issue.

::: commit-message-7a18e:3
(Diff revision 4)
> +With the MediaRecorderErrorEvent now being fired upon async errors the existing
> +tests need to be updated to reflect this.

s/upon/with/ I think?

::: dom/media/test/test_mediarecorder_creation_fail.html:24
(Diff revision 4)
>    var stream = new AudioContext().createMediaStreamDestination().stream;
>    var mediaRecorder = new MediaRecorder(stream);
>  
>    mediaRecorder.onerror = function (e) {
>      is(callbackStep, 0, 'should fired onstop callback');
> -    is(e.name, 'GenericError', 'error name should be GenericError');
> +    is(e.error.name, 'AbortError', 'error name should be AbortError');

UnknownError

::: dom/media/test/test_mediarecorder_creation_fail.html:24
(Diff revision 4)
> -    is(e.name, 'GenericError', 'error name should be GenericError');
> +    is(e.error.name, 'AbortError', 'error name should be AbortError');
>      is(mediaRecorder.mimeType, '', 'mimetype should be empty');
>      is(mediaRecorder.state, 'recording', 'state is recording');

There appears to be a discrepancy between how our mediaRecorder works here and the spec, since the spec says the state should be "inactive" here.

With Chrome and Firefox the only implementations, we should double-check how Chrome behaves here. If both implementations agree, we should consider whether updating the spec instead would be preferable.

::: dom/media/test/test_mediarecorder_getencodeddata.html:52
(Diff revision 4)
> -        is(evt.name, 'GenericError',
> -           'Event name is GenericError');
> +        ok(evt.error instanceof DOMException,
> +          'Events fired from onerror should have a DOMException in their error member');

We should check evt.error.name and evt.error.message as well.

::: dom/media/test/test_mediarecorder_principals.html:94
(Diff revision 4)
>        hasStopped = new Promise(resolve => rec.onstop = resolve);
>        video.play();
>      })
> -    .then(() => ok(true, msgNoThrow), e => is(e.name, null, msgNoThrow))
> +    .then(() => ok(true, msgNoThrow), e => is(e.error.name, null, msgNoThrow))
>      .then(() => Promise.race([
> -      new Promise((_, reject) => rec.onerror = e => reject(new DOMException("", e.name))),
> +      new Promise((_, reject) => rec.onerror = e => reject(new DOMException("", e.error.name))),

No need to create a new DOMException here anymore.

::: dom/media/test/test_mediarecorder_unsupported_src.html:45
(Diff revision 4)
>            } catch(e) {
>              ok(false, 'Should not get exception in pause call.');
>            }
>          }
>          ok(callbackStep < 3, 'onerror callback fired as expected.');
> -        is(e.name, 'GenericError', 'Error name should be GenericError.');
> +        is(e.error.name, 'AbortError', 'Error name should be AbortError.');

UnknownError
Attachment #8894287 - Flags: review?(jib) → review-
(Assignee)

Comment 29

2 years ago
mozreview-review-reply
Comment on attachment 8894285 [details]
Bug 1275856 - Fire MediaRecorderErrorEvent in media recorder.

https://reviewboard.mozilla.org/r/165370/#review171014

> You've added two new errors here. Why?
> 
> According to the spec [1], the only two errors that can be fired are "SecurityError", and "UnknownError" for everything else.
> 
> This means any gecko-specifc debug info about the actual failure (like the aRv number) would need to be captured in the error.message field instead.
> 
> [1] https://w3c.github.io/mediacapture-record/MediaRecorder.html#dom-mediarecorder-start

I'd assumed based on the previous errors being raised in the recorder and that since we call NotifyError outside of start that it was possible to expose any of the exceptions detailed in the spec[1] on the error event. Will revise. Should we remove the possibility of other operations firing an error event?

[1] https://www.w3.org/TR/mediastream-recording/#exception-summary

> Missing the stack part.

I've got a further patch which I'll add to the review containing the changes for this.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 33

2 years ago
mozreview-review-reply
Comment on attachment 8894287 [details]
Bug 1275856- Update tests to reflect new MediaRecorderErrorEvent.

https://reviewboard.mozilla.org/r/165374/#review171020

> s/upon/with/ I think?

I'm trying to say that we'll fire the event in response to an error above, rather than wrapping an error. I will reword this to make both cases more clear.

> There appears to be a discrepancy between how our mediaRecorder works here and the spec, since the spec says the state should be "inactive" here.
> 
> With Chrome and Firefox the only implementations, we should double-check how Chrome behaves here. If both implementations agree, we should consider whether updating the spec instead would be preferable.

Shall we follow this up in another bug?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 37

2 years ago
mozreview-review
Comment on attachment 8895154 [details]
Bug 1275856 - Capture MediaRecorder DOMExceptions early in order to capture JS traces.

https://reviewboard.mozilla.org/r/166286/#review171500

::: dom/media/MediaRecorder.cpp:1147
(Diff revision 1)
>    }
>  
>    MOZ_ASSERT(mSessions.Length() > 0);
>    nsresult rv = mSessions.LastElement()->Pause();
>    if (NS_FAILED(rv)) {
> +    InitializeDomExceptions();

Is this sensible? Would we rather not re-init here and let the exceptions only be set in start? Should we not be throwing here all together (the spec doesn't specify the error event as coming from Pause. Right now it's possible to not have the start Exception present when reaching this code, which is part of why I'm doing the init here.

I've also got guards in the throwing code, with some asserts. There's some overlap with this code in terms of defensive programming. Figured I'd go conservative initially and we can back off as appropriate.

::: dom/media/MediaRecorder.cpp:1166
(Diff revision 1)
>    }
>  
>    MOZ_ASSERT(mSessions.Length() > 0);
>    nsresult rv = mSessions.LastElement()->Resume();
>    if (NS_FAILED(rv)) {
> +    InitializeDomExceptions();

As with pause comment.

::: dom/media/MediaRecorder.cpp:1183
(Diff revision 1)
>      return;
>    }
>    MOZ_ASSERT(mSessions.Length() > 0);
>    nsresult rv = mSessions.LastElement()->RequestData();
>    if (NS_FAILED(rv)) {
> +    InitializeDomExceptions();

As with pause comment.

Comment 38

2 years ago
mozreview-review
Comment on attachment 8894285 [details]
Bug 1275856 - Fire MediaRecorderErrorEvent in media recorder.

https://reviewboard.mozilla.org/r/165370/#review171516
Attachment #8894285 - Flags: review?(jib) → review+

Comment 39

2 years ago
mozreview-review-reply
Comment on attachment 8895154 [details]
Bug 1275856 - Capture MediaRecorder DOMExceptions early in order to capture JS traces.

https://reviewboard.mozilla.org/r/166286/#review171500

> Is this sensible? Would we rather not re-init here and let the exceptions only be set in start? Should we not be throwing here all together (the spec doesn't specify the error event as coming from Pause. Right now it's possible to not have the start Exception present when reaching this code, which is part of why I'm doing the init here.
> 
> I've also got guards in the throwing code, with some asserts. There's some overlap with this code in terms of defensive programming. Figured I'd go conservative initially and we can back off as appropriate.

We only get here if state == Recording, which is only set from start(), so the exception should be present. I'd remove it.

> As with pause comment.

ditto

> As with pause comment.

ditto

Comment 40

2 years ago
mozreview-review
Comment on attachment 8895154 [details]
Bug 1275856 - Capture MediaRecorder DOMExceptions early in order to capture JS traces.

https://reviewboard.mozilla.org/r/166286/#review171524

::: dom/media/MediaRecorder.cpp:1006
(Diff revision 2)
> +  // Make sure the exceptions are initialized in case we error before we get
> +  // another chance.
> +  InitializeDomExceptions();

Can this happen? I'm a bit concerned about the overhead of creating these already, so I'm hoping it can't.
Attachment #8895154 - Flags: review?(jib) → review-

Comment 41

2 years ago
mozreview-review
Comment on attachment 8894287 [details]
Bug 1275856- Update tests to reflect new MediaRecorderErrorEvent.

https://reviewboard.mozilla.org/r/165374/#review171534

::: dom/media/test/test_mediarecorder_getencodeddata.html:54
(Diff revisions 4 - 6)
> +        is(evt.error.name, 'UnknownError', 'Error name should be UnknownError.');
> +        is(evt.error.message, 'The operation failed for an unknown transient reason');

Do we want to test that e.stack points to start() in this file? It's not spec'ed, but we should probably cover it so it won't break on us.
Attachment #8894287 - Flags: review?(jib) → review+
(Assignee)

Comment 42

2 years ago
mozreview-review-reply
Comment on attachment 8895154 [details]
Bug 1275856 - Capture MediaRecorder DOMExceptions early in order to capture JS traces.

https://reviewboard.mozilla.org/r/166286/#review171500

> We only get here if state == Recording, which is only set from start(), so the exception should be present. I'd remove it.

Roger. I'll change the assertions in NotifyError into logs. It's possible right now for the same Exception type to happen multiple times for one call of start(), see `test_mediarecorder_unsupported_src.html`. The first time the Exception is thrown it will be nulled, and then the next time it will need to be reinitialized.

I think this is not what should happen per spec. As you've indicated in another comment, we're not moving the recorder to inactive state at times were it appears we should. I'm going to follow up on this in another bug rather than complicate this bug by bringing that work in.
(Assignee)

Comment 43

2 years ago
mozreview-review-reply
Comment on attachment 8895154 [details]
Bug 1275856 - Capture MediaRecorder DOMExceptions early in order to capture JS traces.

https://reviewboard.mozilla.org/r/166286/#review171524

> Can this happen? I'm a bit concerned about the overhead of creating these already, so I'm hoping it can't.

With the defensive initialization code at the time the event is fired this can be removed anyway, so will get rid of it. Per the spec I agree it shouldn't happen, will test as such once I've made the alterations.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 46

2 years ago
mozreview-review
Comment on attachment 8895266 [details]
Bug 1275856 - Expand MediaRecorder tests to check stack traces when MediaRecorderErrorEvent is fired.

https://reviewboard.mozilla.org/r/166426/#review171590

::: dom/media/test/test_mediarecorder_creation_fail.html:25
(Diff revision 1)
>    var mediaRecorder = new MediaRecorder(stream);
>  
>    mediaRecorder.onerror = function (e) {
>      is(callbackStep, 0, 'should fired onstop callback');
>      is(e.error.name, 'UnknownError', 'error name should be UnknownError');
> +    ok(e.error.stack.includes('test_mediarecorder_creation_fail.html'),

In my local tests I wasn't getting the `start` function name logged in the trace. Correct line number, but function not listed. I'm not sure if this is a known issue. However, it's meant I've made these checks a bit less strict than they could be.

Comment 47

2 years ago
mozreview-review
Comment on attachment 8895154 [details]
Bug 1275856 - Capture MediaRecorder DOMExceptions early in order to capture JS traces.

https://reviewboard.mozilla.org/r/166286/#review171892

Looks good. Kinda wish we still had the assert, but this is probably more robust against future changes.

::: dom/media/MediaRecorder.cpp:1419
(Diff revisions 2 - 3)
>      default:
> -      MOZ_ASSERT(mUnkownDomException);
>        if (!mUnkownDomException) {
> +        LOG(LogLevel::Debug, ("MediaRecorder.NotifyError: "
> +          "mUnkownDomException was not initialized"));
>          mUnkownDomException = DOMException::Create(NS_ERROR_DOM_UNKNOWN_ERR);
>        }
>        init.mError = mUnkownDomException.forget();

Just wondering, would it be helpful for debugging to get the real aRv number value into the UnknownError's message?

We could probably just subclass DOMException here to get access to the protected member.

That, or maybe log it (unless errors are logged where they're thrown)?
Attachment #8895154 - Flags: review?(jib) → review+

Comment 48

2 years ago
mozreview-review
Comment on attachment 8895266 [details]
Bug 1275856 - Expand MediaRecorder tests to check stack traces when MediaRecorderErrorEvent is fired.

https://reviewboard.mozilla.org/r/166426/#review171894
Attachment #8895266 - Flags: review?(jib) → review+
(Assignee)

Comment 49

2 years ago
mozreview-review-reply
Comment on attachment 8894287 [details]
Bug 1275856- Update tests to reflect new MediaRecorderErrorEvent.

https://reviewboard.mozilla.org/r/165374/#review171020

> Shall we follow this up in another bug?

https://bugzilla.mozilla.org/show_bug.cgi?id=1388909 raised to track this.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 52

2 years ago
mozreview-review-reply
Comment on attachment 8895154 [details]
Bug 1275856 - Capture MediaRecorder DOMExceptions early in order to capture JS traces.

https://reviewboard.mozilla.org/r/166286/#review171892

> Just wondering, would it be helpful for debugging to get the real aRv number value into the UnknownError's message?
> 
> We could probably just subclass DOMException here to get access to the protected member.
> 
> That, or maybe log it (unless errors are logged where they're thrown)?

Added logging here (also realized I've typoed mUnknownDomException, so have fixed that up).

Comment 53

2 years ago
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 6 changesets with 14 changes to 8 files
remote: 
remote: WebIDL file dom/webidl/RecordErrorEvent.webidl altered in changeset 0271692503de without DOM peer review
remote: WebIDL file dom/webidl/MediaRecorderErrorEvent.webidl altered in changeset 5d2156c05a51 without DOM peer review
remote: 
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: 
remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote: 
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.d_webidl hook failed
abort: push failed on remote
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 60

2 years ago
mozreview-review
Comment on attachment 8894284 [details]
Bug 1275856 - Add MediaRecorderErrorEvent webidl files and update moz.build.

https://reviewboard.mozilla.org/r/165368/#review172262
Attachment #8894284 - Flags: review?(bugs) → review+

Comment 61

2 years ago
mozreview-review
Comment on attachment 8894286 [details]
Bug 1275856 - Remove now unused RecrodErrorEvent.

https://reviewboard.mozilla.org/r/165372/#review172264

Hopefully we won't break websites with these backwards incompatible changes :/
Attachment #8894286 - Flags: review?(bugs) → review+

Comment 62

2 years ago
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/407dc5f8de5b
Add MediaRecorderErrorEvent webidl files and update moz.build. r=jib,smaug
https://hg.mozilla.org/integration/autoland/rev/1c8e6dd99100
Fire MediaRecorderErrorEvent in media recorder. r=jib
https://hg.mozilla.org/integration/autoland/rev/38739c5a3ae8
Remove now unused RecrodErrorEvent. r=jib,smaug
https://hg.mozilla.org/integration/autoland/rev/210e8d6d5f85
Update tests to reflect new MediaRecorderErrorEvent. r=jib
https://hg.mozilla.org/integration/autoland/rev/2fb8d23f1ece
Capture MediaRecorder DOMExceptions early in order to capture JS traces. r=jib
https://hg.mozilla.org/integration/autoland/rev/d229d269ac55
Expand MediaRecorder tests to check stack traces when MediaRecorderErrorEvent is fired. r=jib
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 70

2 years ago
mozreview-review
Comment on attachment 8896531 [details]
Bug 1275856 - Update dom event tests to reflect removal of RecorderErrorEvent and new MediaRecorderErrorEvent.

https://reviewboard.mozilla.org/r/167796/#review173390
Attachment #8896531 - Flags: review?(bugs) → review+

Comment 71

2 years ago
mozreview-review
Comment on attachment 8896532 [details]
Bug 1275856 - Update expected outcomes of webplatform test for MediaRecorder.

https://reviewboard.mozilla.org/r/167798/#review174076
Attachment #8896532 - Flags: review?(jib) → review+

Comment 72

2 years ago
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ebe132a416d4
Add MediaRecorderErrorEvent webidl files and update moz.build. r=jib,smaug
https://hg.mozilla.org/integration/autoland/rev/2f9f9eab06b2
Fire MediaRecorderErrorEvent in media recorder. r=jib
https://hg.mozilla.org/integration/autoland/rev/39674f7a8148
Remove now unused RecrodErrorEvent. r=jib,smaug
https://hg.mozilla.org/integration/autoland/rev/3c105bee024c
Update tests to reflect new MediaRecorderErrorEvent. r=jib
https://hg.mozilla.org/integration/autoland/rev/2d3c818bf51d
Capture MediaRecorder DOMExceptions early in order to capture JS traces. r=jib
https://hg.mozilla.org/integration/autoland/rev/c735325c95be
Expand MediaRecorder tests to check stack traces when MediaRecorderErrorEvent is fired. r=jib
https://hg.mozilla.org/integration/autoland/rev/0153cadbabe3
Update dom event tests to reflect removal of RecorderErrorEvent and new MediaRecorderErrorEvent. r=smaug
https://hg.mozilla.org/integration/autoland/rev/f51e7690796d
Update expected outcomes of webplatform test for MediaRecorder. r=jib
(Assignee)

Updated

2 years ago
Flags: needinfo?(bvandyk)
Duplicate of this bug: 970772

Comment 75

2 years ago
Commits pushed to master at https://github.com/mdn/kumascript

https://github.com/mdn/kumascript/commit/abc0e9e088a97e80b3ee91689d7a6624a8ac839c
Fix bug 1275856 - added MediaRecorderErrorEvent

Added this interface to the list of interfaces under the
MediaStream Recording API.

https://github.com/mdn/kumascript/commit/ef840bf177023ba454ab65527c493ae517921da0
Merge Fix bug 1275856 - added MediaRecorderErrorEvent to interface list (#339)

Fix bug 1275856 - added MediaRecorderErrorEvent
You need to log in before you can comment on or make changes to this bug.