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

Reject unless fully-active and has focus. #192

Merged
merged 2 commits into from
Dec 2, 2021
Merged

Conversation

jan-ivar
Copy link
Member

@jan-ivar jan-ivar commented Sep 16, 2021

Fixes #191.


Preview | Diff

@eladalon1983
Copy link
Member

eladalon1983 commented Sep 16, 2021

getUserMedia permissions can be persisted; getDisplayMedia permissions can't. If the tab is not visible, this PR produces no observable difference in behavior. If the tab is visible but unfocused (for example, two browser windows visible on the screen side by side), this would produce the difference of not invoking a prompt on the unfocused browser window+tab (until focused).

  1. Did I get it right?
  2. Are we sure this is really preferable? It makes more sense to me in the gUM case, because of the persisted permissions. Here, it seems to me that only showing the media picker when the user focuses the window, produces a more surprising behavior for the user.

@jan-ivar
Copy link
Member Author

If the tab is not visible, this PR produces no observable difference in behavior.

Did I get it right?

Not quite. There is no visibility requirement (largely because that's hard to define), so this PR affects tabs regardless of their visibility. There's a similar-sounding active document test but that's on documents inside a tab (think BFCache), so it doesn't ensure the tab itself is visible. Alas, keyboard focus is the best we have until whatwg/html#6211 is solved.

this would produce the difference of not invoking a prompt on the unfocused browser window+tab (until focused).

Correct.

Are we sure this is really preferable?

The problem isn't specific to unprompted access. Without this PR (or something like it), nothing prevents a browser from throwing up a prompt on behalf of a background tab, which is what happened in crbug 920733.

It makes more sense to me in the gUM case, because of the persisted permissions.

Please ★ crbug 1163972.

@jan-ivar
Copy link
Member Author

Also, this issue seems rather moot for getDisplayMedia which does require transient activation, which is hard to get without focus, so we're only talking about edge cases where the user defocuses a window before the prompt has a chance to appear.

@eladalon1983
Copy link
Member

Not quite. There is no visibility requirement

I know visibility is not mentioned by the PR. I was saying that there are two scenarios in which the PR's effect can be considered - tab invisible or visible.

  1. Tab invisible: if the user has tabbed away (ctrl+tab), then the prompt would appear immediately when the user tabs back. The user has no idea if the prompt was there all along, and therefore the PR "produces no observable difference in behavior."
  2. Tab visible-but-unfocused (cited example - side-by-side windows): The PR produces behavior which could even be seen as surprising for the user. Or not. Discussion invited.

crbug 920733

Thanks for bringing this one to my attention; I see what appears to be a lingering implementation issue there - that an inactive-but-visible Chrome tab can steal focus from another window using getDisplayMedia(). Which leads me to my point -

nothing prevents a browser from throwing up a prompt on behalf of a background tab

If the prompt is non-intrusive - does not steal focus and does not peek out of the borders of the window - then it is a non-issue.
And I think the prompt should be non-intrusive. I think we'd be right to specify that.
Note that a bugged implementation that can steal focus is still susceptible to timing edge cases, like when - gDM check for is-focused executes, user alt+tabs away, application steals focus back.

this issue seems rather moot

If that is the case, I'd prefer a shorter spec over a long one.
Requiring (i) transient activation and (ii) a non-intrusive prompt seem to me like a preferable way to go. Wdyt?

@jan-ivar
Copy link
Member Author

jan-ivar commented Sep 17, 2021

This PR restores spec-mandated behavior that was lost by mistake, making crbug 920733 a spec violation again, like it was at the time. There's no record of this regression being intentional, so fixing it seems editorial. I suggest we merge this and discuss the problem you raised in w3c/mediacapture-main#752 (comment) since it's not unique to gDM.

nothing prevents a browser from throwing up a prompt on behalf of a background tab

If the prompt is non-intrusive - does not steal focus and does not peek out of the borders of the window - then it is a non-issue.

This is exactly what crbug 920733 did, and it was bad, because users could mistake the request as coming from the current tab, not a background tab.

a non-intrusive prompt

Feel free to open a new issue, but note there's no precedent for specs mandating intrusiveness of prompts, and such a spec would likely be anything but short.

@eladalon1983
Copy link
Member

This PR restores spec-mandated behavior that was lost by mistake

Could you please clarify what was lost and why it should be restored? Reading #132 does not make it as obvious to me. Both the pre-PR and post-PR texts discuss rejecting getDisplayMedia() if called without user-interaction. The post-PR text lets the UA allow gDM in the split second right after the user interacts with the page (it's unclear to me what the pre-PR text specifies here). Is this really a problem?

making crbug 920733 a spec violation again

Isn't the requirement for transient activation sufficient here? (Modulo the aforementioned timing issue.)

Feel free to open a new issue, but note there's no precedent for specs mandating intrusiveness of prompts, and such a spec would likely be anything but short.

I think it would be useful to at least say that the dialog shown to the user may not steal away focus/activation from anything other than the current browsing context. The user can always alt+tab to another native app while the UA is still processing the already-approved call to getDisplayMedia, and that this would result in the alt+tab being undone is unfortunate and preventable. I don't think #192 prevents that (due to timing issues), so would be good to generally say that the browser should employ prompts and dialogs that do not risk introducing such issues at the edges.

@jan-ivar
Copy link
Member Author

Could you please clarify what was lost and why it should be restored?

gDM used to be built on top of gUM (inheriting its focus requirement), until I split gDM out into its own algorithm in #73.

But since gDM, ulike gUM, required "triggered by user activation" (v1), gUM's focus test seemed redundant — after all, gDM had to be called directly or indirectly by a relevant user input event handler, e.g. a mouse click, so it would have focus — so we removed the redundancy using editorial discretion.

Is this really a problem?

Yes the v2 model is time-based and no longer requires focus, which regressed our assumptions, and the language is no longer redundant, so we need to put it back to plug the hole.

making crbug 920733 a spec violation again

Isn't the requirement for transient activation sufficient here? (Modulo the aforementioned timing issue.)

The timing issue could be exploited, e.g. invoke gDM from window.beforeunload to show the prompt in the context of a different (more trusted) tab, like it could in crbug 920733. This PR plugs the timing hole.

the dialog shown to the user may not steal away focus/activation from anything other than the current browsing context.

That wouldn't be a normative MAY. Replacing normative language with strong recommendation would weaken the requirement, which doesn't seem desirable to me.

@eladalon1983
Copy link
Member

eladalon1983 commented Sep 27, 2021

To summarize our discussion of the matter last Thursday:

  • @jan-ivar says this was a pre-existing spec mandate that was unintentionally removed, and so does not require consensus to reinstate. As @alvestrand has been here longer than me and knows the history better, I request that he review this PR, in case he remembers some hisotrical context which I was not here in time to witness.
  • I see two general problems, one atop the other, and I think they persist despite this change:
    i. Corner cases with timing can occur even with the amended wording, as the test for has focus can be executed a moment before focus is lost, but before the user-prompt is displayed. This is only a problem because:
    ii. The spec does not prevent getDisplayMedia's user-prompts from stealing focus away from other browser surfaces and/or native apps.
    iii. @jan-ivar mentioned that mandating UX behavior in specs is hard. I think it may be possible to achieve here by not talking of UX in the spec, but rather of activation and focus, which are are well-defined (if poorly understood) concepts. My proposal is to mandate that gDM's user-prompt MUST NOT change the activation/focus status of other browser contexts. (Admittedly, this approach does not deal with native apps. Maybe speaking of not gaining activation again could help?)
  • I think a fix of (ii) would render (i) a non-issue, and would at the same time obviate the need for this PR. If @jan-ivar thinks it would be better to treat this as a follow-up, I totally understand.

@jan-ivar jan-ivar changed the title Wait for fully-active and has focus. Reject unless fully-active and has focus. Oct 12, 2021
@jan-ivar
Copy link
Member Author

@youennf @eladalon1983 I've updated the PR based on our discussion last Editor's meeting, to reject instead of wait. PTAL.

@jan-ivar jan-ivar merged commit 7f88822 into w3c:gh-pages Dec 2, 2021
@jan-ivar jan-ivar deleted the hasfocus branch December 2, 2021 16:00
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.

Restore focus requirement that got lost in #132
2 participants