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

Define CaptureController.setFocusBehavior() #240

Merged
merged 67 commits into from Oct 13, 2022
Merged
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
9143a5c
Define CaptureController.setFocusBehavior()
eladalon1983 Oct 5, 2022
9d3d932
Add TODO for task
eladalon1983 Oct 5, 2022
caa0c98
Add task
eladalon1983 Oct 5, 2022
e33458c
Avoid "set it back"
eladalon1983 Oct 5, 2022
ab9a59a
Change error types
eladalon1983 Oct 5, 2022
56eb1d2
Fix isBound oopsie
eladalon1983 Oct 5, 2022
175ad5e
Refer to the argument
eladalon1983 Oct 6, 2022
1771128
Fix unintended assumption
eladalon1983 Oct 6, 2022
e5022db
s/browser/display
eladalon1983 Oct 6, 2022
9498e58
Move the rule about initialFocusLost
eladalon1983 Oct 6, 2022
d35b1e8
Update index.html
eladalon1983 Oct 6, 2022
a25e292
Change to source
eladalon1983 Oct 7, 2022
29b6f67
Merge branch 'cond2' of https://github.com/eladalon1983/mediacapture-…
eladalon1983 Oct 7, 2022
cb90ee1
Continued
eladalon1983 Oct 7, 2022
cba6d19
Update index.html
eladalon1983 Oct 7, 2022
8eb6140
Use a table
eladalon1983 Oct 7, 2022
ae0bb29
Link task
eladalon1983 Oct 7, 2022
c111550
Update index.html
eladalon1983 Oct 10, 2022
a679e42
No wrap
eladalon1983 Oct 10, 2022
4c453dd
move
eladalon1983 Oct 10, 2022
30d9b4f
Merge branch 'w3c:gh-pages' into cond2
eladalon1983 Oct 10, 2022
626770b
nit
eladalon1983 Oct 10, 2022
2f93cb9
Merge branch 'cond2' of https://github.com/eladalon1983/mediacapture-…
eladalon1983 Oct 10, 2022
76e917c
In parallel
eladalon1983 Oct 10, 2022
f8fb1bd
Update index.html
eladalon1983 Oct 11, 2022
1a9b4d1
Update index.html
eladalon1983 Oct 11, 2022
b45cc3e
Update index.html
eladalon1983 Oct 11, 2022
46cccef
Update index.html
eladalon1983 Oct 11, 2022
872f8a0
InitialFocusLost comment
eladalon1983 Oct 11, 2022
ad4ab9f
Merge branch 'cond2' of https://github.com/eladalon1983/mediacapture-…
eladalon1983 Oct 11, 2022
030f18b
Update index.html
eladalon1983 Oct 11, 2022
fdab181
Update index.html
eladalon1983 Oct 11, 2022
4105b52
Update index.html
eladalon1983 Oct 11, 2022
2acb011
Update index.html
eladalon1983 Oct 11, 2022
b36a47e
InvalidStateError
eladalon1983 Oct 11, 2022
f427948
Merge branch 'cond2' of https://github.com/eladalon1983/mediacapture-…
eladalon1983 Oct 11, 2022
16ade48
Fix
eladalon1983 Oct 11, 2022
075115d
first
eladalon1983 Oct 11, 2022
333b0f7
Losing focus
eladalon1983 Oct 11, 2022
9b1dfb1
indent
eladalon1983 Oct 11, 2022
aa8f42a
wip
eladalon1983 Oct 11, 2022
2d0256b
nit
eladalon1983 Oct 11, 2022
8259423
Fix unintentional edit
eladalon1983 Oct 11, 2022
33f356e
Revert "Fix unintentional edit"
eladalon1983 Oct 11, 2022
12d9854
nits
eladalon1983 Oct 11, 2022
f95635f
s/steps/step
eladalon1983 Oct 13, 2022
43d8346
Update index.html
eladalon1983 Oct 13, 2022
e5ff0a6
Update index.html
eladalon1983 Oct 13, 2022
6d185b7
in parallel
eladalon1983 Oct 13, 2022
606a8c4
Merge branch 'cond2' of https://github.com/eladalon1983/mediacapture-…
eladalon1983 Oct 13, 2022
430378b
controller.
eladalon1983 Oct 13, 2022
2aeccb8
Move busy-wait guard
eladalon1983 Oct 13, 2022
f759c31
Set [[FocusBehavior]]
eladalon1983 Oct 13, 2022
4ddd9ad
nit
eladalon1983 Oct 13, 2022
ec84ef6
{{CaptureController/[[FocusChangeDisabled]]}}
eladalon1983 Oct 13, 2022
fd547b0
Merge branch 'w3c:gh-pages' into cond2
eladalon1983 Oct 13, 2022
915875d
Fix closer of <var>
eladalon1983 Oct 13, 2022
9f2eebf
Merge branch 'cond2' of https://github.com/eladalon1983/mediacapture-…
eladalon1983 Oct 13, 2022
1926f0b
Merge branch 'w3c:gh-pages' into cond2
eladalon1983 Oct 13, 2022
7cb1236
Fix
eladalon1983 Oct 13, 2022
32d1a96
Merge branch 'w3c:gh-pages' into cond2
eladalon1983 Oct 13, 2022
43ad5b7
Merge branch 'cond2' of https://github.com/eladalon1983/mediacapture-…
eladalon1983 Oct 13, 2022
8d1a8c0
no-focus-change op
eladalon1983 Oct 13, 2022
b5a6f8f
Fix dl/p issue
eladalon1983 Oct 13, 2022
ce4a025
Fix ol
eladalon1983 Oct 13, 2022
242391e
Update index.html
eladalon1983 Oct 13, 2022
d0d2ce8
Update index.html
eladalon1983 Oct 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
316 changes: 290 additions & 26 deletions index.html
Expand Up @@ -201,16 +201,6 @@ <h2>
method is called, the user agent MUST run the following
steps:</p>
<ol>
<li>
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the diff-tool just thinks this was removed. In effect, I just put something ahead of it and then fixed the indentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I see, the controller association moved up. However, as I mentioned this still isn't going to guarantee the controller is always associated on failure e.g. if the WebIDL binding code throws TypeErrors on the constructor arguments. Are we OK with that?

Would it be more logical to move it down instead, after input validation, just ahead of the in-parallel steps, since it has side-effects? That way, if the returned promise is pending (as determined with Promise.race), the app can know the binding happened or not (something we can WPT test).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the mental model for the developer should be kept as simple as possible. "I can call setFocusBehavior() multiple times before gDM is called, then exactly one more time, at latest when gDM's promise resolves" - this is already at the limit. I don't want to add the caveat of "...unless gDM fails, in which case I can call it again."

As a developer, if your gDM call fails, try again from scratch.

It's a shame that failures over WebIDL bindings don't get captured here, but I see it as an edge-case we can avoid worrying about. I don't think programs would typically run into this and then try to resolve such an error on the fly, programmatically. Rather, it will be a bug that someone will manually fix.

<p>If the [=relevant global object=] of [=this=] does not have
[=transient activation=], return a promise <a>rejected</a>
with a {{DOMException}} object whose {{DOMException/name}}
attribute has the value {{InvalidStateError}}.</p>
</li>
<li>
<p>Let <var>options</var> be the method's first
argument.</p>
</li>
<li>
<p>
Let <var>controller</var> be <var>options</var>.<code>controller</code> if present,
Expand All @@ -219,20 +209,36 @@ <h2>
</li>
<li>
<p>
If controller is not <code>null</code>, run the following steps:
If <var>controller</var> is not <code>null</code>, run the following steps:
</p>
<ol>
<li>
If <var>controller</var>.{{CaptureController/[[isBound]]}}
<ol>
<li>
<p>
If <var>controller</var>.{{CaptureController/[[IsBound]]}}
is <code>true</code>, return a promise [=reject|rejected=] with
a {{DOMException}} object whose {{DOMException/name}} attribute has
the value {{InvalidStateError}}.
</li>
<li>
Set <var>controller</var>.{{CaptureController/[[isBound]]}} to <code>true</code>.
</li>
</p>
</li>
<li>
<p>
Set <var>controller</var>.{{CaptureController/[[IsBound]]}} to <code>true</code>.
eladalon1983 marked this conversation as resolved.
Show resolved Hide resolved
</p>
</li>
</ol>
</li>
<li>
<p>
If the [=relevant global object=] of [=this=] does not have
[=transient activation=], return a promise <a>rejected</a>
with a {{DOMException}} object whose {{DOMException/name}}
attribute has the value {{InvalidStateError}}.
</p>
</li>
<li>
<p>Let <var>options</var> be the method's first
argument.</p>
</li>
<li>
<p>Let <var>constraints</var> be
<code>[</code><var>options</var>.<code>audio</code>,
Expand Down Expand Up @@ -293,7 +299,7 @@ <h2>
<p>Let <var>p</var> be a new promise.</p>
</li>
<li>
<p>Run the following steps in parallel:</p>
<p>Run the following steps [=in parallel=]:</p>
<ol>
<li>
<p>For each media type <var>T</var> in
Expand Down Expand Up @@ -383,9 +389,39 @@ <h2>
<var>message</var>)</code>.</p>
</li>
<li>
<p><a>Resolve</a> <var>p</var> with <var>stream</var> and
abort these steps. This invocation of {{MediaDevices/getDisplayMedia()}}
is now considered to have produced a new <dfn>capture-session</dfn></p>
<p>
This invocation of {{MediaDevices/getDisplayMedia()}} is now
considered to have produced a new <dfn>capture-session</dfn>.
</p>
</li>
<li>
If <var>controller</var> is not <code>null</code>, run the following steps:
<ol>
<li>
<p>
Set <var>controller</var>.{{CaptureController/[[Source]]}}
to <var>stream</var>'s video track's <a data-cite="GETUSERMEDIA#dfn-source-0">[[\Source]]</a>.
</p>
</li>
<li>
<p>
Set <var>controller</var>.{{CaptureController/[[DisplaySurfaceType]]}} to the
to <var>stream</var>'s video track's {{DisplayCaptureSurfaceType}}.
</p>
</li>
<li>
<p>
Queue a task to run the [=finalize focus decision algorithm=]
on <var>controller</var>.
</p>
</li>
</ol>
</li>
<li>
<p>
<a>Resolve</a> <var>p</var> with <var>stream</var> and
abort these steps.
</p>
</li>
<li>
<p><em>Permission Failure</em>: [=Reject=]
Expand All @@ -399,6 +435,23 @@ <h2>
<p>Return <var>p</var>.</p>
</li>
</ol>
<p>
When the top-level document loses focus, run the following steps on all
{{CaptureController}} objects in that document and in documents of its nested
[=browsing contexts=]:
</p>
<ol>
<li>
<p>
If {{CaptureController/[[Source]]}} is <code>undefined</code>, abort these steps.
</p>
</li>
<li>
<p>
Set {{CaptureController/[[FocusChangeDisabled]]}} to <code>true</code>.
</p>
</li>
</ol>
<p>
The <a>user agent</a> MUST NOT capture content that's behind a
partially transparent captured <a>display surface</a>.
Expand Down Expand Up @@ -743,6 +796,46 @@ <h2>
</p>
</div>
</section>
<section>
<h2><dfn>CaptureStartFocusBehavior</dfn></h2>
<p>
Describes whether an application invoking {{CaptureController/setFocusBehavior()}}
would like the user agent to focus the [=display surface=] associated with
that {{CaptureController}}'s [=capture-session=].
</p>
<pre class="idl">
enum CaptureStartFocusBehavior {
"focus-captured-surface",
"no-focus-change"
};
</pre>
<table data-dfn-for="CaptureStartFocusBehavior" class="simple">
<tbody>
<tr>
<th colspan="2">
Enumeration description
</th>
</tr>
<tr>
<td style="white-space:nowrap">
<dfn id="idl-def-CaptureStartFocusBehavior.focus-captured-surface">focus-captured-surface</dfn>
</td>
<td>
The application prefers that the [=display surface=] associated with
this {{CaptureController}}'s [=capture-session=] be focused.
</td>
</tr>
<tr>
<td>
<dfn id="idl-def-CaptureStartFocusBehavior.no-focus-change">no-focus-change</dfn>
</td>
<td>
The application prefers that the user agent not change focus.
</td>
</tr>
</tbody>
</table>
</section>
<section>
<h2><dfn>CaptureController</dfn></h2>
<p>
Expand All @@ -761,20 +854,191 @@ <h2><dfn>CaptureController</dfn></h2>
[Exposed=Window, SecureContext]
interface CaptureController {
constructor();
// TODO: Add setFocusBehavior() in a separate PR.
undefined setFocusBehavior(CaptureStartFocusBehavior focusBehavior);
};
</pre>
<dl data-link-for="CaptureController" data-dfn-for="CaptureController" class="methods">
<dt>
<dfn>constructor</dfn>
</dt>
<dd>
eladalon1983 marked this conversation as resolved.
Show resolved Hide resolved
Create a new {{CaptureController}} object with the following internal slots:
<table class="simple">
<thead>
<tr>
<th>Internal Slot</th>
<th>Initial value</th>
<th>Description (<em>non-normative</em>)</th>
</tr>
</thead>
<tbody>
<tr>
<td><dfn>[[\IsBound]]</dfn></td>
<td><code>false</code></td>
<td>
Whether an application has attempted to associate [=this=]
with a [=capture-session=].
</td>
</tr>
<tr>
<td><dfn>[[\Source]]</dfn></td>
<td><code>null</code></td>
<td>
The source of the associated [=capture-session=].
</td>
</tr>
<tr>
<td><dfn>[[\DisplaySurfaceType]]</dfn></td>
<td><code>null</code></td>
<td>
Once capture starts, this will be set to the type of the captured [=display surface=].
</td>
</tr>
<tr>
<td><dfn>[[\FocusChangeDisabled]]</dfn></td>
<td><code>false</code></td>
<td>
Whether focus-change has been disabled by an external event
or a user agent consideration.
</td>
</tr>
<tr>
<td><dfn>[[\FocusDecisionFinalized]]</dfn></td>
<td><code>false</code></td>
<td>
Set to true when the focus decision is finalized.
</td>
</tr>
<tr>
<td><dfn>[[\FocusBehavior]]</dfn></td>
<td><code>null</code></td>
<td>
The focus behavior desired by the application.
</td>
</tr>
</tbody>
</table>
<p>
The user agent MAY set {{CaptureController/[[FocusChangeDisabled]]}}
to <code>true</code> at any moment based on its own logic.
</p>
</dd>
<dt>
<dfn>setFocusBehavior</dfn>
</dt>
<dd>
<p>
Create one internal slot: <dfn data-dfn-for="CaptureController">[[\isBound]]</dfn>,
initialized to <code>false</code>.
Run the following steps:
</p>
<ol>
<li>
<p>
Let <var>focusBehavior</var> be the method's first argument.
</p>
</li>
<li>
<p>
If {{CaptureController/[[Source]]}} is <code>null</code>,
set {{CaptureController/[[FocusBehavior]]}} to <var>focusBehavior</var>
and abort these steps.
eladalon1983 marked this conversation as resolved.
Show resolved Hide resolved
</p>
</li>
<li>
<p>
If {{CaptureController/[[Source]]}} has been
<a data-cite="GETUSERMEDIA#source-stopped">stopped</a>,
[=exception/throw=] an "{{InvalidStateError}}" {{DOMException}}.
</p>
</li>
<li>
<p>
If {{CaptureController/[[DisplaySurfaceType]]}} is neither
{{DisplayCaptureSurfaceType/"browser"}} nor {{DisplayCaptureSurfaceType/"window"}},
[=exception/throw=] an "{{InvalidStateError}}" {{DOMException}}.
Comment on lines +955 to +957
Copy link
Member

Choose a reason for hiding this comment

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

First thought: this seems like an error of type, not state. Maybe TypeError and move it up one step?

Second thought: the "type" varies by what the user chooses in the picker... should we abort silently instead? Seems a footgun otherwise: Developer tests their code without error, but forgot to manually test choosing "Entire desktop" in the prompt which will throw in production.

Also, "Entire desktop" is already "in focus' so to speak, so why not call this success instead of failure?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are we OK with that?

I've commented on that elsewhere. The comment threads are getting out of hand. I propose that we merge this and iterate. If there are substantive issues to resolve first, like methods vs. attribute, let's focus on them and leave uncontroversial spec-language nits for a follow-up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Second thought: the "type" varies by what the user chooses in the picker... should we abort silently instead?

We do abort silently if setFocusBehavior() was called before gDM, when the surface type was unknown. If the call is made after the user makes their choice, failing to check the surface type is a Web-dev bug and should be fixed. By failing silently we would be delaying the time when the Web-dev would realize their mistake.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, "Entire desktop" is already "in focus' so to speak, so why not call this success instead of failure?

That depends on the OS and whether it's the current desktop or not.
At any rate, I really think this is all fine to discuss over v2.

</p>
</li>
<li>
<p>
If {{CaptureController/[[FocusDecisionFinalized]]}} is <code>true</code>,
[=exception/throw=] an "{{InvalidStateError}}" {{DOMException}}.
</p>
</li>
<li>
<p>
Set {{CaptureController/[[FocusBehavior]]}} to <var>focusBehavior</var>.
</p>
</li>
<li>
<p>
Run the [=finalize focus decision algorithm=] on [=this=].
</p>
</li>
</ol>
</dd>
</section>
</dl>
<p>
The <dfn>finalize focus decision algorithm</dfn>, given a
<var>controller</var>, consists of running
the following steps:
</p>
<ol>
<li>
<p>
If <var>controller</var>.{{CaptureController/[[FocusDecisionFinalized]]}}
is <code>true</code>, abort these steps.
</p>
</li>
<li>
<p>
Set <var>controller</var>.{{CaptureController/[[FocusDecisionFinalized]]}}
to <code>true</code>.
</p>
</li>
<li>
<p>
If <var>controller</var>.{{CaptureController/[[FocusChangeDisabled]]}}
is <code>true</code>, abort these steps.
</p>
</li>
<li>
<p>
If too much time has elapsed since the [=capture-session=] started,
the user agent SHOULD set {{CaptureController/[[FocusDecisionFinalized]]}}
to <code>true</code>. The timespan is left up to the user agent,
but it is recommended that a value of one second be used.
</p>
</li>
<li>
<p>
If <var>controller</var>.{{CaptureController/[[DisplaySurfaceType]]}} is neither
{{DisplayCaptureSurfaceType/"browser"}} nor {{DisplayCaptureSurfaceType/"window"}},
abort these steps.
</p>
</li>
<li>
<p>
Run the following step [=in parallel=]:
</p>
</li>
<ol>
<li>
<p>
If <var>controller</var>.{{CaptureController/[[FocusBehavior]]}}
is {{CaptureStartFocusBehavior/"no-focus-change"}},
focus the [=display surface=] representing the capturing document.
</p>
</li>
<li>
<p>
If <var>controller</var>.{{CaptureController/[[FocusBehavior]]}}
is {{CaptureStartFocusBehavior/"focus-captured-surface"}},
focus the [=display surface=] referred to by
<var>controller</var>.{{CaptureController/[[Source]]}}.
</p>
</li>
</ol>
</ol>
</section>
<section>
<h2><dfn>SelfCapturePreferenceEnum</dfn></h2>
<p>
Expand Down