Skip to content

Make Error Event go after ondataavailable. #118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Make Error Event go after ondataavailable. #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
52df3be
On Error, first fire an Event called ondataavailable, then an Event c…
yell0wd0g Feb 14, 2017
d8c7a2e
correc link
yell0wd0g Feb 14, 2017
43f20b4
correct link
yell0wd0g Feb 14, 2017
8d6477f
Merge branch 'gh-pages' into prA03__send_error_event_after_ondataavai…
yell0wd0g Feb 14, 2017
d73b629
made #error-handling-principles more normative
yell0wd0g Feb 15, 2017
82bc34b
Merge branch 'prA03__send_error_event_after_ondataavailable' of githu…
yell0wd0g Feb 15, 2017
6068d71
@foolip comments
yell0wd0g Feb 15, 2017
ce88b78
s/<var>event</var>/{{UnknownError}}/
yell0wd0g Feb 16, 2017
96db655
Correct <a>fire an error event</a> call sites
yell0wd0g Feb 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 31 additions & 20 deletions MediaRecorder.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ interface MediaRecorder : EventTarget {
runs the following steps:
<ol>
<li>Set {{state}} to {{inactive}}.</li>
<li><a>Fire an error event</a> named {{SecurityError}} at
<var>target</var>.</li>
<li><a>Fire an error event</a> named <a>error</a> with a
{{SecurityError}} at <var>target</var>.</li>
<li><a href="#to-fire-a-blob-event">Fire a blob event</a> named
<a>dataavailable</a> at <var>target</var> with <var>blob</var>.</li>
<li><a>Fire an event</a> named <a>stop</a> at <var>target</var>.</li>
Expand All @@ -222,8 +222,8 @@ interface MediaRecorder : EventTarget {
following steps:
<ol>
<li>Set {{state}} to {{inactive}}.</li>
<li><a>Fire an error event</a> named {{UnknownError}} at
<var>target</var>.</li>
<li><a>Fire an error event</a> named <a>error</a> with an
{{UnknownError}} at <var>target</var>.</li>
Copy link
Member

@jan-ivar jan-ivar Jan 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should fix the event order here instead.

It looks like these are the normative steps we already have for recording errors, here in step 5.4 under the start method: "If the UA at any point is unable to continue gathering data for reasons other than isolation properties or stream track set, it MUST stop gathering data, and queue a task, using the DOM manipulation task source, that runs the following steps".

<li><a href="#to-fire-a-blob-event">Fire a blob event</a> named
<a>dataavailable</a> at <var>target</var> with <var>blob</var>.</li>
<li><a>Fire an event</a> named <a>stop</a> at <var>target</var>.</li>
Expand Down Expand Up @@ -286,7 +286,7 @@ interface MediaRecorder : EventTarget {
<td class="prmNullFalse"><span role="img" aria-label="False">&#10008;</span></td>
<td class="prmOptTrue"><span role="img" aria-label="True">&#10004;</span></td>
<td class="prmDesc">The minimum number of milliseconds of data
to return in a single Blob.</td>
to return in a single {{Blob}}.</td>
</tr>
</tbody>
</table>
Expand All @@ -302,7 +302,7 @@ interface MediaRecorder : EventTarget {
task, using the DOM manipulation task source, that runs the following steps:
<ol>
<li>Set {{state}} to {{inactive}} and stop gathering data.</li>
<li>Let <var>blob</var> be the Blob of collected data so far and
<li>Let <var>blob</var> be the {{Blob}} of collected data so far and
let <var>target</var> be the MediaRecorder context object, then
<a href="#to-fire-a-blob-event">fire a blob event</a> named
<a>dataavailable</a> at <var>target</var> with <var>blob</var>.</li>
Expand Down Expand Up @@ -369,7 +369,7 @@ interface MediaRecorder : EventTarget {
(Note that <var>blob</var> will be empty if no data has been gathered
yet.)</li>

<li>Create a new Blob and gather subsequent data into it.</li>
<li>Create a new {{Blob}} and gather subsequent data into it.</li>
</ol>
</li>
<li>return <code>undefined</code>.</li>
Expand Down Expand Up @@ -569,19 +569,30 @@ dictionary BlobEventInit {
# Error handling # {#error-handling}

## General principles ## {#error-handling-principles}
<em>This section is non-normative.</em>

The UA will throw a {{DOMException}} when the error can be detected at the time
that the call is made. In all other cases the UA will <a>fire an event</a> named
{{MediaRecorderErrorEvent}}. If recording has been started and not yet stopped
when the error occurs, let <var>blob</var> be the {{Blob}} of collected data so
far; after raising the error, the UA will <a href="#to-fire-a-blob-event">fire a
dataavailable event</a> with <var>blob</var>; immediately after the UA will then
<a>fire an event</a> named <code>stop</code>.
The UA may set platform-specific limits, such as those for the minimum and
maximum {{Blob}} size that it will support, or the number of
{{MediaStreamTrack}}s it will record at once.
It will signal a fatal error if these limits are exceeded.

If at any moment the UA finds an error condition, the UA MUST queue a task,
using the DOM manipulation task source, that runs the following steps:
Copy link
Member

@jan-ivar jan-ivar Jan 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry this is a bit broad.

But more importantly, it looks like we already have normative steps for this as mentioned above, so with this PR there would be two competing normative steps for this condition, which I think is confusing (do we fire twice?) - I'd vote we fix the existing place instead and leave General Principles non-normative or remove it, it seems a bit redundant.

<ol>
<li>Set {{state}} to {{inactive}} and stop gathering data.</li>

<li>Let <var>blob</var> be the {{Blob}} of collected data so far and
let <var>target</var> be the MediaRecorder context object</li>

<li><a href="#to-fire-a-blob-event">Fire a blob event</a> named
<a>dataavailable</a> at <var>target</var> with <var>blob</var>.</li>

<li><a>Fire an event</a> named <a>stop</a> at <var>target</var></li>

<li><a>Fire an error event</a> named <a>error</a> with an {{UnknownError}} at
Copy link
Member

@jan-ivar jan-ivar Jan 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Do we really want to fire stop before error? It would make it harder to detect errors IMHO. Most code I've seen seem to treat reaching stop without error as success, and this would break that assumption.

How about this order: dataavailable, error, stop?

<var>target</var>.</li>
</ol>

<div class="note">
The UA MAY set platform-specific limits, e.g. for the minimum and maximum
{{Blob}} size that it will support, or the number of {{MediaStreamTrack}}s it
will record at once. It will signal a fatal error if these limits are
exceeded.
</div>

## MediaRecorderErrorEvent ## {#errorevent-section}

Expand Down
Loading