-
Notifications
You must be signed in to change notification settings - Fork 62
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 text on whether context object is allowed to request access. #427
Conversation
664a4b0
to
36c8de1
Compare
@annevk can you say if this text looks appropriate? |
As I said in the referenced issue, @zcorpan is changing how this works. If we make But overall this looks okay, except that "this context object" should just be "context object". |
@@ -3578,8 +3576,14 @@ | |||
succeed.</p> | |||
</li> | |||
<li> | |||
<p>If the current [[!HTML51]] Document's <a>user media | |||
enabled flag</a> is unset, return a promise rejected with a | |||
<p>If this <a href="https://dom.spec.whatwg.org/#context-object"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should say
If the current settings object's responsible document is not allowed to use the feature indicated by attribute name allowusermedia, then throw a "SecurityError" DOMException.
See https://w3c.github.io/browser-payment-api/#constructor
(In particular, MediaDevices
is not a node, so "node document" is undefined.)
The allowed to use check needs to be in navigator.getUserMedia() as well, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
navigator.getUserMedia() is a shell around navigator.mediaDevices.getUserMedia(), so no.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zcorpan I like your proposal to use "settings object" and "responsible document" rather than "context object" and "node document".
One thing I was thinking about when doing this though was if we (some time) would allow a worker to use gUM. Would it still be clear what the "responsible document" is?
I updated as proposed by @zcorpan in https://github.com/w3c/mediacapture-main/pull/427/files/36c8de191cef114cf52964b3e35d2f2cb6140d47#r95865625 with except for that I kept it at returning a rejected promise rather than throwing (we did discuss and agree that is the right way, right?). |
Oops. Yeah that was sloppy copy-paste on my part. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve of content, with nit.
<p>The term <dfn><a href="https://html.spec.whatwg.org/multipage/embedded-content.html#allowed-to-use"> | ||
allowed to use</a></dfn> is defined in WHATWG (note we don't have a proper | ||
ref added, in addition it seems HTML5.1 will(?) add defintion | ||
of this term).</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest removing parenthesis before merging.
allowed to use</a></dfn> is defined in WHATWG (note we don't have a proper | ||
ref added, in addition it seems HTML5.1 will(?) add defintion | ||
of this term).</p> | ||
allowed to use</a></dfn> is defined in WHATWG.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably say WHATWG HTML
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, added that.
A first attempt on #268.