Consume user activation in `element.requestFullscreen()` #153

foolip · 2019-05-28T12:54:01Z

Depends on whatwg/html#3851.

Fixes #152.

At least two implementers are interested (and none opposed):
- Chromium: Consume user activation in element.requestFullscreen() #153 (comment)
- Gecko: Consume user activation in element.requestFullscreen() #153 (comment)
Tests are written and can be reviewed and commented upon at:
- Test that element.requestFullscreen() consumes user activation web-platform-tests/wpt#36969
Implementation bugs are filed:
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1804635
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=247920
MDN issue is filed: Document user activation requirement of the Fullscreen API mdn/content#22303

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

mustaqahmed · 2019-05-28T13:42:59Z

Thanks, this looks good from user activation perspective.

LanWei22 · 2019-05-29T18:53:09Z

web-platform-tests/wpt#16758 has been merged into wpt repository.
Three new tests about this behavior are now in https://w3c-test.org/fullscreen/api/
element-request-fullscreen-twice-manual.tentative.html
element-request-fullscreen-two-elements-manual.tentative.html
element-request-fullscreen-two-iframes-manual.tentative.html

mustaqahmed · 2019-12-10T16:13:55Z

fullscreen.bs

@@ -270,6 +270,9 @@ when invoked, must run these steps:
   <li><p>This algorithm is <a>allowed to request fullscreen</a>.
  </ul>

+ <li><p>If <var>error</var> is false, then <a>consume user activation</a> for


FYI: This should now point to the newly added definition of consumption. Not sure why the PR preview doesn't show a link.

Looks like the preview is correctly pointing to the HTML spec link now.

mustaqahmed

I wanted to approve this PR, but apparently I don't have permission to do that! @domenic?

fullscreen.bs

domenic · 2022-11-01T07:55:51Z

@foolip, can we help move this along? I'm happy to rebase it and also take care of any changes after #208.

This would probably solve the issue @marcoscaceres raises in web-platform-tests/wpt#36727 of giving a cross-browser way of consuming user activation.

marcoscaceres · 2022-11-01T08:40:18Z

I guess it depends on what features/API calls follow a call to .requestFullScreen()... I see there is mention of Pointerlock in the bug and Screen Orientation Lock is another.

I'm simultaneously hoping we can prevent the Screen Orientation API (which depends on FS API) from arbitrarily changing the screen orientation without a user gesture.

If you have an Android device handy, try this out in Chrome:
https://marcoscaceres.github.io/playground/annoying.html

(if you can't try it out, it loops rapidly changing the screen orientation making it difficult of users to access the browsers UI - you need to either turn off the screen or quickly swipe from the bottom to stop it).

I personally don't think the above behavior should be acceptable for any API to do on the Web. So, I was hoping we wouldn't consume the user activation in FS API, but rather have the Screen Orientation API consume the user activation.

Alternatively, we could have it consume the user activation if #186 was in place, all in one go. The advantage being that it wouldn't breaking any sites that are depending on FS API to not consuming the user activation.

For pointerlock, maybe a solution like #186 could also work. You request them all at once, and exiting fulls screen releases all the locks.

EdgarChen · 2022-11-01T15:52:55Z

Currently, Gecko also consume the user activation as we had received a bug of trapping the user in fullscreen mode, which invites DOS and spoofing possibilities. And for pointer lock, we allow it without a vaild user activation if document is in fullscreen mode.

marcoscaceres · 2022-11-01T20:30:41Z

Discussing with my WebKit colleagues... but if both Gecko and Chromium consume it, we may follow suit. Let me get back to you.

And for pointer lock, we allow it without a vaild user activation if document is in fullscreen mode.

That is contrary to what is in the Pointer Lock spec, right? Why don't we just link the actions together via a mechanism like in #186?

(also, we should really rewrite the pointer lock spec to integrate properly with the Full Screen API)

fullscreen.bs

mustaqahmed · 2022-11-02T16:00:08Z

Currently, Gecko also consume the user activation as we had received a bug of trapping the user in fullscreen mode, which invites DOS and spoofing possibilities.

Chrome originally proposed this consumption behavior to prevent a spoof, see https://crbug.com/852645.

And for pointer lock, we allow it without a vaild user activation if document is in fullscreen mode.

That is contrary to what is in the Pointer Lock spec, right? Why don't we just link the actions together via a mechanism like in #186?

IIRC Chrome added a similar hack to support some compat argument, can't recall any details! Yes, #186 would be nice.

(Edited to fix the bug link)

EdgarChen · 2022-11-02T16:30:11Z

#186 sounds reasonable to me.

marcoscaceres · 2022-11-02T21:58:54Z

@EdgarChen, if we go with something like #207 (new dictionary member), would you be willing to remove the pointerLock workaround in Firefox? That way, we can just have a consistent model across three specs.

Similar question for Chrome folks. I'm willing to spec that up if everyone is supportive.

EdgarChen · 2022-11-03T09:35:29Z

@EdgarChen, if we go with something like #207 (new dictionary member), would you be willing to remove the pointerLock workaround in Firefox?

To clarify this a bit more: This is not a workaround added for consuming user activation, we behave the same all the way back to Firefox 22 at least (before the current user activation model in spec is proposed). I am worried that removing this would cause compatibility problem. :(

marcoscaceres · 2022-11-11T20:00:28Z

@foolip, could you kindly please update the PR to use the "new" PR template? (I don't have the ability to edit the top comment)

Also, will you be updating any affected tests?

foolip · 2022-11-14T12:07:26Z

@marcoscaceres there are 3 tests in wpt that are tentative due to #152:

https://github.com/web-platform-tests/wpt/blob/master/fullscreen/api/element-request-fullscreen-twice-manual.tentative.html
https://github.com/web-platform-tests/wpt/blob/master/fullscreen/api/element-request-fullscreen-two-elements-manual.tentative.html
https://github.com/web-platform-tests/wpt/blob/master/fullscreen/api/element-request-fullscreen-two-iframes-manual.tentative.html

But I think with consuming, this can be reduced to just the first test, ensuring the second call is rejected.

fullscreen.bs

marcoscaceres · 2022-11-14T23:03:49Z

Filed WebKit bug:
https://bugs.webkit.org/show_bug.cgi?id=247920

Thanks for the details about the tests, @foolip. If those can get updated and changed to non-tentative, that would be great.

This makes a bunch of other tests redundant, as they're also calling requestFullscreen() twice, and should all fail in exactly the same way. element-request-fullscreen-same-manual.html is also converted to ensure that a second call in a new click does work, to ensure the failure isn't because the element is the same as the fullscreen element. Follows whatwg/fullscreen#153.

upsuper · 2022-11-15T12:16:47Z

I have a feeling that there is nothing wrong to allow requestFullscreen without user activation when the document is already in fullscreen, or is entering fullscreen (by a previous call). Though to be honest I'm not sure whether it is useful to allow that either.

This makes a bunch of other tests redundant, as they're also calling requestFullscreen() twice, and should all fail in exactly the same way. element-request-fullscreen-same-manual.html is also converted to ensure that a second call in a new click does work, to ensure the failure isn't because the element is the same as the fullscreen element. Follows whatwg/fullscreen#153.

foolip · 2022-11-15T12:51:35Z

@upsuper I think you're right about that, since the page script already has full control over what's being presented and could replace all the contents of the current fullscreen element. However, that'd be a change from the currently implemented behavior, and I'd prefer to handle that separately from this. Can you file an issue if you think we should change that?

foolip · 2022-11-15T12:52:01Z

@marcoscaceres test PR is up and the whole PR template is filled in now.

marcoscaceres · 2022-11-15T21:14:03Z

Thanks @foolip! Everything looks great.

fullscreen.bs

https://bugs.webkit.org/show_bug.cgi?id=247920 rdar://102355401 Reviewed by Youenn Fablet. Matches Chrome & Firefox implementations, we should reset the activation timestamps when requesting fullscreen. This disallows double non-user initiated requestFullscreen calls. Spec PR: whatwg/fullscreen#153 WPT upstream revision: web-platform-tests/wpt@897b406 * LayoutTests/imported/w3c/web-platform-tests/fullscreen/api/element-request-fullscreen-consume-user-activation-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/fullscreen/api/element-request-fullscreen-consume-user-activation.html: Added. * LayoutTests/imported/w3c/web-platform-tests/fullscreen/api/element-request-fullscreen-same-element-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/fullscreen/api/element-request-fullscreen-same-element.html: Added. * LayoutTests/imported/w3c/web-platform-tests/fullscreen/api/w3c-import.log: * LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/fullscreen/api/element-request-fullscreen-same-element-expected.txt: Added. * Source/WebCore/dom/FullscreenManager.cpp: (WebCore::FullscreenManager::requestFullscreenForElement): Canonical link: https://commits.webkit.org/257554@main

marcoscaceres · 2022-12-09T12:25:18Z

This is now also implemented in WebKit.

@foolip, anything left to get this merged?

Depends on whatwg/html#3851. Fixes #152.

Suggestion applied.

foolip · 2023-05-03T07:24:53Z

@marcoscaceres sorry I totally dropped the ball here. I've updated the PR and all I need now is review from someone with write access. Paging @annevk, as @domenic seems to be OOO today.

annevk

This seems fine. The step immediately preceding this could do with some love at some point. The booleans are still hurting me.

foolip · 2023-05-03T07:32:07Z

Thanks @annevk! Let me try inverting the preceding steps and see if you like that.

foolip force-pushed the consume-user-activation branch from a8e5d45 to a07aff5 Compare May 28, 2019 12:56

foolip changed the title ~~Consume user gesture in element.requestFullscreen()~~ Consume user activation in element.requestFullscreen() May 28, 2019

foolip mentioned this pull request May 28, 2019

Consume user gestures in requestFullscreen #152

Closed

foolip force-pushed the consume-user-activation branch from a07aff5 to e4b2cc3 Compare May 28, 2019 13:28

foolip mentioned this pull request Aug 21, 2019

Added a new section on User Activation v2. whatwg/html#3851

Merged

mustaqahmed reviewed Dec 10, 2019

View reviewed changes

annevk mentioned this pull request Jan 29, 2020

Review Draft Publication: January 2020 #162

Closed

Base automatically changed from master to main January 15, 2021 07:53

mustaqahmed reviewed May 18, 2022

View reviewed changes

domenic previously requested changes May 18, 2022

View reviewed changes

fullscreen.bs Outdated Show resolved Hide resolved

foolip force-pushed the consume-user-activation branch from e4b2cc3 to 5bd6d6f Compare November 2, 2022 15:01

foolip requested a review from domenic November 2, 2022 15:04

foolip commented Nov 2, 2022

View reviewed changes

fullscreen.bs Outdated Show resolved Hide resolved

mustaqahmed mentioned this pull request Nov 2, 2022

User gestures web-platform-tests/interop#142

Closed

foolip commented Nov 14, 2022

View reviewed changes

fullscreen.bs Outdated Show resolved Hide resolved

foolip mentioned this pull request Nov 15, 2022

Test that element.requestFullscreen() consumes user activation web-platform-tests/wpt#36969

Merged

foolip mentioned this pull request Nov 15, 2022

Document user activation requirement of the Fullscreen API mdn/content#22303

Closed

foolip added the impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation label Nov 15, 2022

michaelwasserman mentioned this pull request Nov 15, 2022

Allow fullscreen on already fullscreen elements without user activation #193

Closed

foolip mentioned this pull request Nov 18, 2022

RFC 128: Consume User Activation web-platform-tests/rfcs#128

Merged

marcoscaceres reviewed Nov 28, 2022

View reviewed changes

fullscreen.bs Outdated Show resolved Hide resolved

michaelwasserman mentioned this pull request Nov 28, 2022

Consume user activation w3c/screen-orientation#218

Open

4 tasks

nt1m mentioned this pull request Dec 8, 2022

Make requestFullscreen() consume user activation WebKit/WebKit#7306

Merged

foolip force-pushed the consume-user-activation branch from 616380c to 50786c2 Compare May 3, 2023 07:16

Consume user activation in element.requestFullscreen()

e1ab822

Depends on whatwg/html#3851. Fixes #152.

foolip force-pushed the consume-user-activation branch from 50786c2 to e1ab822 Compare May 3, 2023 07:19

annevk approved these changes May 3, 2023

View reviewed changes

foolip merged commit b3bab96 into main May 3, 2023

foolip deleted the consume-user-activation branch May 3, 2023 07:57

foolip mentioned this pull request May 3, 2023

Remove top layer definitions, now that CSS Position 4 contains them. #223

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consume user activation in `element.requestFullscreen()` #153

Consume user activation in `element.requestFullscreen()` #153

foolip commented May 28, 2019 •

edited by pr-preview bot

Loading

mustaqahmed commented May 28, 2019

LanWei22 commented May 29, 2019

mustaqahmed Dec 10, 2019

mustaqahmed May 18, 2022

mustaqahmed left a comment

domenic commented Nov 1, 2022

marcoscaceres commented Nov 1, 2022

EdgarChen commented Nov 1, 2022

marcoscaceres commented Nov 1, 2022

mustaqahmed commented Nov 2, 2022 •

edited

Loading

EdgarChen commented Nov 2, 2022

marcoscaceres commented Nov 2, 2022

EdgarChen commented Nov 3, 2022

marcoscaceres commented Nov 11, 2022 •

edited

Loading

foolip commented Nov 14, 2022

marcoscaceres commented Nov 14, 2022

upsuper commented Nov 15, 2022

foolip commented Nov 15, 2022

foolip commented Nov 15, 2022

marcoscaceres commented Nov 15, 2022

marcoscaceres commented Dec 9, 2022

foolip commented May 3, 2023

annevk left a comment

foolip commented May 3, 2023

Consume user activation in element.requestFullscreen() #153

Consume user activation in element.requestFullscreen() #153

Conversation

foolip commented May 28, 2019 • edited by pr-preview bot Loading

mustaqahmed commented May 28, 2019

LanWei22 commented May 29, 2019

mustaqahmed Dec 10, 2019

Choose a reason for hiding this comment

mustaqahmed May 18, 2022

Choose a reason for hiding this comment

mustaqahmed left a comment

Choose a reason for hiding this comment

domenic commented Nov 1, 2022

marcoscaceres commented Nov 1, 2022

EdgarChen commented Nov 1, 2022

marcoscaceres commented Nov 1, 2022

mustaqahmed commented Nov 2, 2022 • edited Loading

EdgarChen commented Nov 2, 2022

marcoscaceres commented Nov 2, 2022

EdgarChen commented Nov 3, 2022

marcoscaceres commented Nov 11, 2022 • edited Loading

foolip commented Nov 14, 2022

marcoscaceres commented Nov 14, 2022

upsuper commented Nov 15, 2022

foolip commented Nov 15, 2022

foolip commented Nov 15, 2022

marcoscaceres commented Nov 15, 2022

marcoscaceres commented Dec 9, 2022

foolip commented May 3, 2023

annevk left a comment

Choose a reason for hiding this comment

foolip commented May 3, 2023

Consume user activation in `element.requestFullscreen()` #153

Consume user activation in `element.requestFullscreen()` #153

foolip commented May 28, 2019 •

edited by pr-preview bot

Loading

mustaqahmed commented Nov 2, 2022 •

edited

Loading

marcoscaceres commented Nov 11, 2022 •

edited

Loading