Test existence and typeof MediaError.message #4638
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
|
Notifying @Ms2ger, @ayg, @gsnedders, @jdm, @jgraham, @sideshowbarker, @zcorpan, and @zqzhang. (Learn how reviewing works.) These tests will be available shortly on w3c-test.org. w3c-test:do-not-mirror (What's this?) |
foolip
approved these changes
Jan 27, 2017
| @@ -28,6 +28,7 @@ | |||
| assert_true(e.error instanceof MediaError); | |||
| assert_equals(e.error.code, 4); | |||
| assert_equals(e.error.code, e.error.MEDIA_ERR_SRC_NOT_SUPPORTED); | |||
| assert_equals(typeof e.error.message, 'string'); | |||
There was a problem hiding this comment.
Since this will fail initially, can you add 'error.message type' as the description (third argument)? That will make the failures more intelligible.
|
Actually, do you think the spec could require a non-empty error message, or will there be errors where we just don't know and would end up having to use "unknown decoder error" or something similarly useless? If it is always possible to have a useful message, then you could also test that the string is non-empty. |
|
@foolip We had discussion already on whether message should be an optional string or not. The consensus was that it was better not to. |
|
@jyavenard I see, there was a bunch of discussion about nullability. And in any event, turns out there's already a PR at whatwg/html#2086 that allows for the empty string, so let's go with that. There's nothing more that could be tested here I think. |
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.
These may currently pass only on Mozilla nightly/aurora builds. Chrome support for MediaError.message is in-progress. @domenic @foolip @jyavenard Please review.
@jdsmith3000, @travisleithead FYI
See related WHATWG spec change whatwg/html#2085 and communication to W3C at https://lists.w3.org/Archives/Public/www-archive/2016Nov/0004.html.