- From: Mike Taylor <notifications@github.com>
- Date: Mon, 20 Nov 2023 14:06:55 -0800
- To: w3c/permissions <permissions@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/permissions/pull/425/review/1740765774@github.com>
@miketaylr requested changes on this pull request.
Looks pretty good, but we do need to fix the markup errors before we can merge.
> + permissions.SetPermissionParameters = {
+ descriptor: permissions.PermissionDescriptor,
+ state: permissions.PermissionState,
+ }
+ </pre>
+ </dd>
+ <dt>Result Type</dt>
+ <dd>
+ <pre class="cddl">
+ EmptyResult
+ </pre>
+ </dd>
+ </dl>
+ <ol algorithm="remote end steps for permissions.setPermission">
+ <p>
+ TODO: Add user <code>context</code> to <code>SetPermissionParameters</code>.
Do you plan to address this TODO in this PR, and the one below?
> +
+ permissions.SetPermissionParameters = {
+ descriptor: permissions.PermissionDescriptor,
+ state: permissions.PermissionState,
+ }
+ </pre>
+ </dd>
+ <dt>Result Type</dt>
+ <dd>
+ <pre class="cddl">
+ EmptyResult
+ </pre>
+ </dd>
+ </dl>
+ <ol algorithm="remote end steps for permissions.setPermission">
+ <p>
nit: `<p>` isn't a valid child of `<ol>`.
> + <pre class="cddl">
+ EmptyResult
+ </pre>
+ </dd>
+ </dl>
+ <ol algorithm="remote end steps for permissions.setPermission">
+ <p>
+ TODO: Add user <code>context</code> to <code>SetPermissionParameters</code>.
+ </p>
+ <p>
+ TODO: Add <code>origin</code>.
+ </p>
+ <p>
+ The [=remote end steps=] with |session| and |command parameters| are:
+ </p>
+ <ol>
nit: `<ol>` isn't a valid child of `<ol>`
> + <li>
+ <a>Queue a task</a> |task| on the [=permissions task source=] of |target|'s
+ [=relevant settings object=]'s [=environment settings object/global object=]'s
+ [=Window/browsing context=] to perform the following step:
+ <ol>
+ <li>Interpret |state| as if it were the
+ result of an invocation of <a>permission state</a> for |descriptor| with the
+ argument |target| made at this moment.
+ </li>
+ </ol>
+ </li>
+ <li>[=list/Append=] |task| to |tasks|.
+ </li>
+ </ol>
+ </li>
+ <li>Wait for all <a>tasks</a> in |tasks| to have executed.
```suggestion
<li>Wait for all <a>tasks</a> in |tasks| to have executed and return.
```
> </p>
+ <section>
+ <h4 id="set-permission-command">
```suggestion
<h4 id="webdriver-set-permission-command">
```
> + parameters|.
+ </li>
+ <li>
+ If |state| is an inappropriate [=permission state=] for any
+ implementation-defined reason, return [=error=] with [=error code=] [=invalid argument=].
+ </li>
+ <li>
+ Let |typedDescriptor| be the object |descriptor| refers to, [=converted to an IDL value=] (|descriptor|, |state|) of
+ {{PermissionSetParameters}} |permission name|'s [=powerful feature/permission descriptor type=].
+ If this conversion throws an exception, return [=error=] with [=error code=] [=invalid argument=].
+ </li>
+ <li>
+ [=Set a permission=] with |typedDescriptor| and |state|.
+ </li>
+ <li>
+ Return [=success=] with data null.
```suggestion
Return [=success=] with data `null`.
```
> + <li>
+ <a>Queue a task</a> |task| on the [=permissions task source=] of |target|'s
+ [=relevant settings object=]'s [=environment settings object/global object=]'s
+ [=Window/browsing context=] to perform the following step:
+ <ol>
+ <li>Interpret |state| as if it were the
+ result of an invocation of <a>permission state</a> for |descriptor| with the
+ argument |target| made at this moment.
+ </li>
+ </ol>
+ </li>
+ <li>[=list/Append=] |task| to |tasks|.
+ </li>
+ </ol>
+ </li>
+ <li>Wait for all <a>tasks</a> in |tasks| to have executed.
maybe?
--
Reply to this email directly or view it on GitHub:
https://github.com/w3c/permissions/pull/425#pullrequestreview-1740765774
You are receiving this because you are subscribed to this thread.
Message ID: <w3c/permissions/pull/425/review/1740765774@github.com>
Received on Monday, 20 November 2023 22:07:01 UTC