Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Contiguous IDL and prose adaptation to match the contemporary style #19

Merged
merged 17 commits into from
Oct 14, 2015

Conversation

anssiko
Copy link
Member

@anssiko anssiko commented Sep 25, 2015

This PR updates the following sections to use Contiguous IDL (Fixes #16) and adapts the prose accordingly:

  • DedicatedWorkerGlobalScope interface
  • VideoMonitorEvent interface
  • VideoProcessorEvent interface

This PR also proposes a normative change to Fix #18, introduces a Terminology section, and does various editorial fixes. I used tidy-html5 to tidy the document, so in that particular commit you see a lot of indentation changes. I used the following config for tidy:

  char-encoding: utf8
  indent: yes
  wrap: 80
  tidy-mark: no

@ChiahungTai @kakukogou Please take a look. I can do a similar editorial pass on the rest of the specification if you like this style.

interface VideoMonitorEvent : Event {
readonly attribute DOMString trackId;
readonly attribute double playbackTime;
readonly attribute ImageBitmap? inputImageBitmap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anssiko, sorry that I do not get the idea of making the inputImageBitmap nullable, could you talk more about this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event attributes should be initialized to something. For symmetry with the output attribute I chose null, but could be something else too. What would you prefer for the default?

This is the usual prose that initializes the value:

The inputImageBitmap attribute must return the value it was initialized to. When the object is created, this attribute must be initialized to null. It represents the ImageBitmap object whose bitmap data is provided by the video monitor source.

In "fire a video worker event named e", when we say in step 1 "create a VideoMonitorEvent", we initialize the attribute to null and in the step 5 that follows to the actual bitmap data.

Similarly to, for example in http://www.w3.org/TR/progress-events/#interface-progressevent

This means that if a web developer programmatically creates a new such event, these are the values the returned object is initialized to. This way the UA retains control over when the implementation will actually prime the input attribute with the real bitmap data (the UA decides when to fire these events). Is that how you intended this to behave? Let me know, and I'll update the PR accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your explanation and I am quite ok with it now.

Is that how you intended this to behave? Let me know, and I'll update the PR accordingly.

Yes, I think it is but I would like to leave this question open for @ChiahungTai to comment on.

@anssiko, just let you know that I am out of office from 9/26 to 10/11, however, I will check messages occasionally, so sorry for my late reply during this period.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I am ok to both the "inputBitmapImage is optional" question and all this PR.

@anssiko
Copy link
Member Author

anssiko commented Oct 7, 2015

@ChiahungTai Please take a look, and let me know if you have any questions.

@ChiahungTai
Copy link
Contributor

@anssiko,
In generally, all looks good to me. I add some suggestions. Please see the comments.
By the way, I am thinking to change the return type of addVideoMonitor/Processor and removeVideoMonitor/Processor to Promise based. Because the implementation of those methods might be async in User Agents. But web developer might want to do some stuffs when the worker really added. So that might make sense to change the return type to Promise based. I will file an issue if you think that make sense. :)

@anssiko
Copy link
Member Author

anssiko commented Oct 12, 2015

@ChiahungTai Thanks for the review! I think I've now addressed your comments. Please feel free to merge if you're happy. I'll do another pass on the rest of the document after this lands.

I think the use of promises makes sense -- we can work on that next.

@anssiko
Copy link
Member Author

anssiko commented Oct 13, 2015

@ChiahungTai I opened #21 for the promise usage and updated this PR with a fix: anssiko@42606a0

Please take a look and feel free to merge this PR, if there are no concerns. I'd like to do an editorial pass on the "ImageBitmap extensions" section in a separate PR, to keep PRs smaller, easier to review.

@kakukogou
Copy link
Contributor

Thank you, @anssiko. I read the whole PR again and have no other concerns. Since @ChiahungTai is also ok to this PR, I will merge it.

kakukogou added a commit that referenced this pull request Oct 14, 2015
Contiguous IDL and prose adaptation to match the contemporary style
@kakukogou kakukogou merged commit 75aceeb into w3c:gh-pages Oct 14, 2015
@anssiko
Copy link
Member Author

anssiko commented Oct 14, 2015

Thanks for your review, @kakukogou and @ChiahungTai.

@anssiko anssiko deleted the contiguous-idl branch October 14, 2015 11:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should there be an onvideomonitor event handler too? Switch to Contiguous IDL
3 participants