Add support for MediaError.message

• Component: Media
• Status: RESOLVED FIXED

[Chris Dumez]

Add support for MediaError.message:
- https://html.spec.whatwg.org/multipage/media.html#mediaerror

Both Chrome and Firefox already support this.

[Chris Dumez]

Created attachment 433634 [details]
Patch

[Alex Christensen]

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?

[Alex Christensen]

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?

[Chris Dumez]

(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?

[Alex Christensen]

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.

[Chris Dumez]

(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.

[Chris Dumez]

Created attachment 433644 [details]
Patch

[Chris Dumez]

Created attachment 433658 [details]
Patch

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/80669971>

[Ahmad Saleem]

*** Bug 170761 has been marked as a duplicate of this bug. ***