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

Updating the spec to match current explainer #135

Merged
merged 1 commit into from
Sep 14, 2016
Merged

Conversation

xxyzzzq
Copy link
Contributor

@xxyzzzq xxyzzzq commented Sep 13, 2016

Please take a look. Though it is still rough now.

@mounirlamouri
Copy link
Member

Given the size of the change, can you provide an HTML output I can look at?

@xxyzzzq
Copy link
Contributor Author

xxyzzzq commented Sep 13, 2016

Promise<void> activate();
Promise<void> deactivate();
partial interface Navigator {
readonly attribute MediaSession? mediaSession;
Copy link
Member

@mounirlamouri mounirlamouri Sep 13, 2016

Choose a reason for hiding this comment

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

Why can MediaSession be null? If it's not null, shouldn't you use [SameObject]? see webidl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@mounirlamouri
Copy link
Member

I think that change is fine. There are some things to fix here and there apart from my comments but better than a spec that no longer match the goal of our work. lgtm! :)

otherwise. On setting, the user agent must run the following steps:
<h2 id="processing-model">Processing model</h2>

The {{MediaSession}} interface is an extension to {{Navigator}}. As there is an
Copy link
Member

Choose a reason for hiding this comment

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

The {{MediaSession}} interface is an extension to {{Navigator}}.

That should be evident from the IDL, so this is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

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

Successfully merging this pull request may close these issues.

3 participants