Closed Bug 1302304 Opened 3 years ago Closed 3 years ago

Remove use of IDL in MediaError object for WebIDL

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: jya, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

Follow up on bug 1299072.

MediaError still uses obsolete IDL method. it should be cleaned up and fully migrated to webidl
Assignee: jyavenard → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8790791 [details] [diff] [review]
Remove IDL bits that reference nsIDOMMediaError; it's not needed anymore

Review of attachment 8790791 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/html/HTMLMediaElement.cpp
@@ +162,5 @@
>  static const double THRESHOLD_HIGH_PLAYBACKRATE_AUDIO = 4.0;
>  // Threshold under which audio is muted
>  static const double THRESHOLD_LOW_PLAYBACKRATE_AUDIO = 0.5;
>  
> +// Media error values.  These need to match the ones in MediaError.webidl.

Can we not generate these from the webidl?  If this was a webidl enum we could use the value directly.  It would be nice if the webidl constants were also available to use in the assignment here or in a static_assert().

::: dom/html/MediaError.cpp
@@ +27,5 @@
>    , mMessage(aMessage)
>  {
>  }
>  
> +void MediaError::GetMessage(nsAString& aResult)

nit: Return type on previous line.

::: dom/html/MediaError.h
@@ +38,5 @@
>    uint16_t Code() const
>    {
>      return mCode;
>    }
> +  void GetMessage(nsAString& aResult);

This can be const now.

nit: Blank line separating this method to match the current file style?
Attachment #8790791 - Flags: review?(bkelly) → review+
> Can we not generate these from the webidl?

For the specific case of numeric ones, we can.  Bug 792059 tracks that.  We should really fix it...  It's mostly old legacy interfaces that have numeric constants, but still.

Will fix the nits; good catch on const.
adding bug to 1302632, as whichever one comes first will bitrot the other
Depends on: 1302632
I have no intention of landing this before bug 1302632 merges to inbound.
Oh, nevermind.  I was thinking of bug 1299072....
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80c000b91b54
Remove IDL bits that reference nsIDOMMediaError; it's not needed anymore.  r=bkelly
https://hg.mozilla.org/mozilla-central/rev/80c000b91b54
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.