Bug 228008 - Add support for MediaError.message
Summary: Add support for MediaError.message
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-15 16:11 PDT by Chris Dumez
Modified: 2021-07-15 23:16 PDT (History)
16 users (show)

See Also:


Attachments
Patch (9.97 KB, patch)
2021-07-15 16:18 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (9.92 KB, patch)
2021-07-15 17:39 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.02 KB, patch)
2021-07-15 22:24 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2021-07-15 16:11:52 PDT
Add support for MediaError.message:
- https://html.spec.whatwg.org/multipage/media.html#mediaerror

Both Chrome and Firefox already support this.
Comment 1 Chris Dumez 2021-07-15 16:18:37 PDT
Created attachment 433634 [details]
Patch
Comment 2 Alex Christensen 2021-07-15 17:26:15 PDT
Comment on attachment 433634 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433634&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:2072
> +    m_error = MediaError::create(MediaError::MEDIA_ERR_SRC_NOT_SUPPORTED, "Unsupported source type"_s);

Do Chrome and Firefox use these same strings?
Comment 3 Alex Christensen 2021-07-15 17:27:20 PDT
Comment on attachment 433634 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433634&action=review

>> Source/WebCore/html/HTMLMediaElement.cpp:2072
>> +    m_error = MediaError::create(MediaError::MEDIA_ERR_SRC_NOT_SUPPORTED, "Unsupported source type"_s);
> 
> Do Chrome and Firefox use these same strings?

Can these strings be derived from m_code instead of storing an additional String an increasing the size of MediaError?
Comment 4 Chris Dumez 2021-07-15 17:29:13 PDT
(In reply to Alex Christensen from comment #3)
> Comment on attachment 433634 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433634&action=review
> 
> >> Source/WebCore/html/HTMLMediaElement.cpp:2072
> >> +    m_error = MediaError::create(MediaError::MEDIA_ERR_SRC_NOT_SUPPORTED, "Unsupported source type"_s);
> > 
> > Do Chrome and Firefox use these same strings?
> 
> Can these strings be derived from m_code instead of storing an additional
> String an increasing the size of MediaError?

While I did the strict minimum in this patch, you could imagine providing more useful error messages in the future. What's the point of having a message attribute if it brings nothing more than the error code attribute?
Comment 5 Alex Christensen 2021-07-15 17:32:20 PDT
Comment on attachment 433634 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433634&action=review

> Source/WebCore/html/MediaError.h:46
> +    static Ref<MediaError> create(Code code, const String& message)

This could at least be an r-value then.
Comment 6 Chris Dumez 2021-07-15 17:32:42 PDT
(In reply to Alex Christensen from comment #5)
> Comment on attachment 433634 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433634&action=review
> 
> > Source/WebCore/html/MediaError.h:46
> > +    static Ref<MediaError> create(Code code, const String& message)
> 
> This could at least be an r-value then.

Sure.
Comment 7 Chris Dumez 2021-07-15 17:39:13 PDT
Created attachment 433644 [details]
Patch
Comment 8 Chris Dumez 2021-07-15 22:24:48 PDT
Created attachment 433658 [details]
Patch
Comment 9 EWS 2021-07-15 23:15:55 PDT
Committed r279978 (239721@main): <https://commits.webkit.org/239721@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433658 [details].
Comment 10 Radar WebKit Bug Importer 2021-07-15 23:16:16 PDT
<rdar://problem/80669971>