Implement HTMLMediaElement.volume attribute

[ferjm]

Spec: https://html.spec.whatwg.org/multipage/media.html#dom-media-volume

[highfive]

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

[lucasfantacuci]

Hi, I'm very new to the open source world.
Though I'm pretty sure that I'll need some mentoring, I'd like to help on this issue.

[lucasfantacuci]

As I could see, I have to change the HTMLMediaElement struct (in file : servo/components/script/dom/htmlmediaelement.rs) by adding this new volume field.

Probably, I'll have to change either the HTMLMediaElement::new_inherited() function that build this object.

Will I have to handle how to retrieve this value?
Or will I have to make a check in this function?

'Cause in the specification says that : "If the new value is outside the range 0.0 to 1.0 inclusive, then, on setting, an "IndexSizeError" DOMException must be thrown instead."

[lucasfantacuci]

@highfive: assign me

[highfive]

Hey @lucasfantacuci! Thanks for your interest in working on this issue. It's now assigned to you!

[jdm]

There will be six parts to this:

• updating the HTMLMediaElement.webidl file (https://html.spec.whatwg.org/multipage/media.html#media-elements:dom-media-volume)
• add a volume member field to the HTMLMediaElement structure which represents the playback volume
• implement the new getter method on HTMLMediaElement that returns this new field
• implement the new setter method on HTMLMediaElement, checking the provided value per the spec
• signal the media player (via this interface) to change the volume
• add support for the new signal in the gstreamer backend

@ferjm will likely need to provide a bit more guidance on the final two steps.

[lucasfantacuci]

Oh, thx Sir. I'm following the guides, and as soon as possible, I gonna create a PR.

[ferjm]

signal the media player (via this interface) to change the volume

You'll need to add a new set_volume function to the Player trait. There are two implementations of this trait: a dummy one, for the platforms where we don't have gstreamer support yet, and the gstreamer one.

For the gstreamer implementation, we have the inner_player_proxy! macro. You can take set_input_size as an example of how to add the new set_volume to the trait implementation and to the PlayerInner struct.

add support for the new signal in the gstreamer backend

The gst-player function that changes the volume is set_volume

[lucasfantacuci]

Sirs, should I create any kind of test for it?

[ferjm]

There are already some Web Platform Tests for this:

• ./mach test-wpt tests/wpt/web-platform-tests/html/semantics/embedded-content/media-elements/audio_volume_check.html
• ./mach test-wpt tests/wpt/web-platform-tests/html/semantics/embedded-content/media-elements/video_volume_check.html
• ./mach tests-wpt tests/wpt/web-platform-tests/html/semantics/embedded-content/media-elements/event_volumechange.html

You will likely need to update the tests expectations since hopefuly all or at least some of these tests will start passing.

[lucasfantacuci]

Hi Sirs, I'm stucked trying to realize how to deal with that:

I changed the .webidl file, so it will execute a script, that enforces me to implement two methods into HTMLMediaElementMethods.

The getter one and the setter one. Here is what the command ./mach check throws me:

Since it says that the SetVolume must return the unit type "()".

How should I check the provided value in this setter?

I was expecting to return something like Err("IndexSizeError"), but I can't return anything.

Therefore, at the player trait I can return a kind of error, so,

Should I code this provided value checking at the interface realization?
If not, how should I handle it?

[jdm]

You will need to aadd the [Throws] annotation to the webidl to allow it to return a Result value that is converted into a JS exception.