Define CaptureController.setFocusBehavior()

[eladalon1983]

Fixes #190.

---

Preview | Diff

[eladalon1983]

Still some work here, but early feedback welcome.

[youennf]

Some comments below.
In general, it seems the spec assumes that the UA is focused at the time of resolution, which might not be the case if the captured window is already focused due to the window selection process.
Also, where is the code stating that if controller is not null, automatic focusing rule is prevented?

[youennf]

At this step, we might want to run steps in parallel, given this might actually happen in another process.

[eladalon1983]

To be sure I understand you correctly, could you clarify the proposed change?

[eladalon1983]

Friendly ping @youennf for this question.

[eladalon1983]

In general, it seems the spec assumes that the UA is focused at the time of resolution, which might not be the case if the captured window is already focused due to the window selection process.

There is a line that should handle this: "If [[initialFocusLost]] is true, abort these steps."
I've referred to it in an in-line comment, I assume we'll continue there.

Also, where is the code stating that if controller is not null, automatic focusing rule is prevented?

The TPAC meeting concluded with this result: "We'll not specify the default behavior (focus/no-focus) for either case (controller/no-controller)."

(That comment was published before the minutes came out, but I first checked with Dom that the summary matched the minutes he was going to publish.)

[eladalon1983]

@jan-ivar and @youennf, this PR is ready for review.

[youennf]

@youennf, I have intentionally shaped the PR to be no-op for your case.

This makes this whole API useless for these UAs, meaning it will not be implemented.
It also casts doubts on the actual API design if it can only accommodate one single selection picker model.

could you please suggest a concrete change

How about the following:

• Rename [InitialFocusLost]] to [[Disabled]] or [[FocusChangeDisabled]] for clarity.
• Make the controller listen to loss of page focus at the time the promise is resolved. If page already lost focus at this point, the controller remains active except if the page regains focus and loses it again.
• Mention that UAs may decide to set [[Disabled]] based on other heuristics.
• Update the finalise focus decision algorithm to call focus on either the capturing surface or the captured surface based on [[FocusBehavior]].

[eladalon1983]

The threads are really getting hard to follow, and the dog ate my homework, in that I have penned a response to one of Jan-Ivar's comments, that I now see was lost - unclear how. Namely, I want to point out that a property involves a getter that is problematic on many accounts, one additional one being that it's unclear what "default" means (recall that the default could be different for tabs/windows, and it's too early to know what the user chose). So before setting, the value is meaningless, and after setting, a getter is not necessary.

I propose that we:

1. Concentrate on aligning the substantive issues. Method vs. property, callability before gDM, etc.
2. Merge this PR.
3. Iterate on spec language.

I think the size and number of the threads make it too difficult to proceed otherwise.

[eladalon1983]

(@youennf) How about the following:
...

Thank you for the concrete proposal. I am happy to adopt a variant of this. However, I am not the only one who will have opinions, and this can take some time (the PR currently adds ~250 lines, and has roughly that many comments). This does not seem to change behavior, only improve the spec, so it should not be a blocker. I can commit to producing the follow-up PR by end-of-day of the day when we merge this PR.

[youennf]

so it should not be a blocker

I think we should demonstrate in this PR that the API is expected to work with more than Chrome current device picker.
This honestly does not seem like a lot of editorial work to achieve this.

[eladalon1983]

This honestly does not seem like a lot of editorial work to achieve this.

The challenge is in the excessive number of open comment threads in case your editorial vision does not immediately align with Jan-Ivar's.

[eladalon1983]

So, I've tried to accommodate this one more round of spec-shape comments. Hopefully we can agree that this is enough for v1, or something very close to it.

• Rename [InitialFocusLost]] to [[Disabled]] or [[FocusChangeDisabled]] for clarity.

I've gone with [[FocusChangeDisabled]] because [[Disabled]] would refer to future APIs too, and we don't intend that atm.

• Make the controller listen to loss of page focus at the time the promise is resolved. If page already lost focus at this point, the controller remains active except if the page regains focus and loses it again.

That's already in the spec - see section with "When the top-level document loses focus, run the following steps on all {{CaptureController}} objects", noting that it's no-op before calling gDM(controller).

• Mention that UAs may decide to set [[Disabled]] based on other heuristics.

I've added that.

• Update the finalise focus decision algorithm to call focus on either the capturing surface or the captured surface based on [[FocusBehavior]].

I don't think focusing the capturing surface is desirable. It could interfere with the users own actions, and cannot otherwise help.

[alvestrand]

@youennf claims that a 2-line change to the PR will address his concern. @eladalon1983 and @youennf will work to finish today.

[eladalon1983]

This is ready. Wdys @youennf?

[eladalon1983]

I see GitHub has had a hiccup when committing the last suggestions, and things somehow got merged without them. I'll fix that later tonight.

[eladalon1983]

@beaufortfrancois, you've mentioned some commits were dropped, but as far as I can see these are in gh-pages. Please advise. (Note that https://github.com/w3c/mediacapture-screen-share/pull/245/files does not appear to be the PR to have fixed this; Youenn's suggestions appear on the left side, too.)