Add a message attribute to MediaError #2086
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
domenic
added
addition/proposal
domenic
removed
the
do not merge yet
foolip
approved these changes
Jan 27, 2017
| agent is able to supply about the cause of the error condition, or the empty string if the user | ||
| agent is unable to supply such details. This message string must not contain only the information | ||
| already available via the supplied error code; for example, it must not simply be a translation of | ||
| the code into a string format. If no additional information is available beyond that provided by |
|
Tests are at #2085 so with a nit fixed there, I think this is all good to go. |
|
So tests are at web-platform-tests/wpt#4638, but not merged yet. It seems two implementers are on board. Seems okay then indeed. |
domenic
removed
the
needs tests
domenic
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jan 27, 2017
MXEBot
pushed a commit
to mirror/chromium
that referenced
this pull request
Apr 22, 2017
This improves the level of information available for web authors to use
when debugging media playback errors.
* Adds support for the MediaError.message field [1] [2]
* Changes MediaLog::PipelineStatusToString() to return just a
stringified version of the enum identifier, suitable for use as a
UA-specific-code when included in MediaError.message.
* Changes MediaLog::GetErrorMessage() to produce messages in the form:
[[Stringified pipeline error code, if any, without any colon][: The
first MEDIA_ERROR_LOG_ENTRY from the log, if any, since that is most
likely to contain the most precise error information]]
The produced message does not contain any newlines.
See the updated media_browsertest.cc for basic examples.
* When the message would otherwise be empty for various cases in
HTMLMediaElement, and more meaningful information beyond the basic
standard MediaError codes and HTMLMediaElement networkStates is
available, reports basic additional information (prefixed by error
code "MEDIA_ELEMENT_ERROR: ".) This should assist edge case
differentiation. See the updated media_browsertest.cc for basic
examples.
* Test coverage:
* Adds basic layout test coverage test similar to that added in [3].
* Adds basic content browser test coverage for a small variety of
errors, since these messages' format and content are specific to
Chrom*.
The format of the message string may change in future (for example, see
discussion in [4]). Also, the variety of error codes in the message
prior to the first ':' will likely change in future, and the descriptive
error message detail following the ':' will likely change to include
more cases or more consistent format in future.
[1] - whatwg/html#2085
[2] - whatwg/html#2086
[3] - web-platform-tests/wpt#4638
[4] - whatwg/html#2531
[Intent to implement and ship] -
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yDIyWozG1xk
BUG=601086, 710617
Review-Url: https://codereview.chromium.org/2660003003
Cr-Commit-Position: refs/heads/master@{#466359}
GnorTech
pushed a commit
to nwjs/chromium.src
that referenced
this pull request
Apr 26, 2017
This improves the level of information available for web authors to use
when debugging media playback errors.
* Adds support for the MediaError.message field [1] [2]
* Changes MediaLog::PipelineStatusToString() to return just a
stringified version of the enum identifier, suitable for use as a
UA-specific-code when included in MediaError.message.
* Changes MediaLog::GetErrorMessage() to produce messages in the form:
[[Stringified pipeline error code, if any, without any colon][: The
first MEDIA_ERROR_LOG_ENTRY from the log, if any, since that is most
likely to contain the most precise error information]]
The produced message does not contain any newlines.
See the updated media_browsertest.cc for basic examples.
* When the message would otherwise be empty for various cases in
HTMLMediaElement, and more meaningful information beyond the basic
standard MediaError codes and HTMLMediaElement networkStates is
available, reports basic additional information (prefixed by error
code "MEDIA_ELEMENT_ERROR: ".) This should assist edge case
differentiation. See the updated media_browsertest.cc for basic
examples.
* Test coverage:
* Adds basic layout test coverage test similar to that added in [3].
* Adds basic content browser test coverage for a small variety of
errors, since these messages' format and content are specific to
Chrom*.
The format of the message string may change in future (for example, see
discussion in [4]). Also, the variety of error codes in the message
prior to the first ':' will likely change in future, and the descriptive
error message detail following the ':' will likely change to include
more cases or more consistent format in future.
[1] - whatwg/html#2085
[2] - whatwg/html#2086
[3] - web-platform-tests/wpt#4638
[4] - whatwg/html#2531
[Intent to implement and ship] -
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yDIyWozG1xk
BUG=601086, 710617
Review-Url: https://codereview.chromium.org/2660003003
Cr-Commit-Position: refs/heads/master@{#466359}
(cherry picked from commit ed8e709)
TBR=jochen@chromium.org,sandersd@chromium.org,mlamouri@chromium.org,mcasas@chromium.org,dalecurtis@chromium.org,foolip@chromium.org
Review-Url: https://codereview.chromium.org/2837133002 .
Cr-Commit-Position: refs/branch-heads/3071@{#173}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
xqq
pushed a commit
to xqq/chromium-media
that referenced
this pull request
May 17, 2017
This improves the level of information available for web authors to use
when debugging media playback errors.
* Adds support for the MediaError.message field [1] [2]
* Changes MediaLog::PipelineStatusToString() to return just a
stringified version of the enum identifier, suitable for use as a
UA-specific-code when included in MediaError.message.
* Changes MediaLog::GetErrorMessage() to produce messages in the form:
[[Stringified pipeline error code, if any, without any colon][: The
first MEDIA_ERROR_LOG_ENTRY from the log, if any, since that is most
likely to contain the most precise error information]]
The produced message does not contain any newlines.
See the updated media_browsertest.cc for basic examples.
* When the message would otherwise be empty for various cases in
HTMLMediaElement, and more meaningful information beyond the basic
standard MediaError codes and HTMLMediaElement networkStates is
available, reports basic additional information (prefixed by error
code "MEDIA_ELEMENT_ERROR: ".) This should assist edge case
differentiation. See the updated media_browsertest.cc for basic
examples.
* Test coverage:
* Adds basic layout test coverage test similar to that added in [3].
* Adds basic content browser test coverage for a small variety of
errors, since these messages' format and content are specific to
Chrom*.
The format of the message string may change in future (for example, see
discussion in [4]). Also, the variety of error codes in the message
prior to the first ':' will likely change in future, and the descriptive
error message detail following the ':' will likely change to include
more cases or more consistent format in future.
[1] - whatwg/html#2085
[2] - whatwg/html#2086
[3] - web-platform-tests/wpt#4638
[4] - whatwg/html#2531
[Intent to implement and ship] -
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yDIyWozG1xk
BUG=601086, 710617
Review-Url: https://codereview.chromium.org/2660003003
Cr-Original-Commit-Position: refs/heads/master@{#466359}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: ed8e7091b5b0a51a8c0781f95813a5929c1f8110
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2085.
Marking "do not merge yet" since the discussion in #2085 is still settling, and we should make sure a wider audience gets a chance to participate before finalizing and merging. Let's have the discussion over there instead of here.