Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: HTML media events and elements #81

Open
wants to merge 2 commits into
base: master
from
Open

WIP: HTML media events and elements #81

wants to merge 2 commits into from

Conversation

@emk
Copy link

@emk emk commented Jan 14, 2018

Not yet ready to merge! I'm working on implementing #62. However, I would be very interested in feedback about how I can make this code fit nicely into stdweb without causing any headaches for the maintainers.

Progress:

  • HTML media events. These are pretty complete at this point (except for some proprietary Mozilla extensions). There are a large number of these events, and they all have nearly-identical implementations, so I decided to implement them using a macro.
  • A basic IHTMLMediaElement trait. This is shared between <audio> and <video>. I've implemented a handful of the most important attributes and methods, but there are plenty more that could be added. Right now, I put this in the html_elements namespace, along with a supporting ReadyState type, but maybe this should live elsewhere?
  • AudioElement and VideoElement structs. These are just implementations of IHTMLMediaElement, and they shoudn't need any logic of their own, as far as I know.
  • Tests. I haven't quite figured out stdweb testing yet, but I need to look at it before I go further.
  • A real-world app. I want to actually try to use this from Rust.
  • More IHTMLMediaElement attributes and methods?

Anyway, thank you for your encouragement, and please let me know if you have any suggestions!

emk added 2 commits Jan 14, 2018
These media events are all nearly identical, with no special payloads,
and there are a lot of them. So at least for the time being, I'm
generating them all using a macro.
We add some basic support for <audio> and <video> elements, but many
attributes have been omitted.
Copy link
Owner

@koute koute left a comment

For now this is looking pretty nice! Thanks for the great work!

One extra thing I'd like to request - in places you can please add links to the specs (as normal comments, not doc comments). In the next few weeks I'm planning to go through the whole stdweb and add them everywhere to make it easier to audit whenever our bindings are correct.

Tests. I haven't quite figured out stdweb testing yet, but I need to look at it before I go further.

You can run them with cargo web test --features web_test, which will run every test we have in headless chromium. Unfortunately you can't test async stuff like that though. (I have some ideas on how to make async tests possible, but I haven't yet gotten to it.)

///
/// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/readyState)
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum ReadyState {

This comment has been minimized.

@koute

koute Jan 15, 2018
Owner

ReadyState -> MediaReadyState maybe? (We have way to many ReadyStates, so I'd like them to all have unique names to reduce confusion.)

This comment has been minimized.

@emk

emk Jan 15, 2018
Author

Great idea! I'll rename it.

@emk
Copy link
Author

@emk emk commented Jan 15, 2018

Thank you for your feedback!

One extra thing I'd like to request - in places you can please add links to the specs (as normal comments, not doc comments).

I've tried to add doc comments to MDN wherever MDN has actual docs. (I omitted some of the individual attribute links for IHtmlMediaElement, because the MDN docs were only available for the interface as a whole.)

Do you have any examples of the kind of comment that you'd like me to add? Do you have any preferences for a particular version of the HTML spec that I should link to?

I think I can provide at least partial test coverage without async tests.

@koute
Copy link
Owner

@koute koute commented Jan 15, 2018

I've tried to add doc comments to MDN wherever MDN has actual docs.

Yep! Those are mostly for the users.

Do you have any examples of the kind of comment that you'd like me to add? Do you have any preferences for a particular version of the HTML spec that I should link to?

You can just put a link to the specs as a normal // comment right before the thing the specs is for.

These are only meant to be for us so it doesn't really matter how exactly it'll look like; it's just nice for it to be there so that when reading the code one can check how it's supposed to behave without having to search for the specs manually.

You can link to the newest version as long as it's official.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.