Assigning to Ojan since almost duplicate blocked bug is assigned to Ojan as well.

I was just about to file a similar bug. This also works on Linux and Windows, but not MacOS.

The link mentioned above is 404. I recreated it in a small HTML file (attached).

Hmm, this is indeed a problem (note there is a workaround which is right-click -> exit fullscreen, or simply Ctrl+W). Not sure why it is RBS given that this isn't a regression (it seems). Arguably this regressed in 49 when we turned off the obvious mouse way to exit, but it still would've been a minor problem before that.

The obvious solution is to impose a minimum time (e.g., 2 seconds) after exiting fullscreen before a site is allowed to re-enter fullscreen. This time should still be quite short in case the user legitimately wants to re-enter fullscreen, but long enough to give the user a chance to close the tab via the mouse before the malicious site takes control again. Anyone have a better suggestion?

(We can also do more complex heuristics like increase the minimum time the more times you go into fullscreen, etc.)

Deleted: fullscreen-trap.html 172 bytes

fullscreen-trap.html 172 bytes View Download

A few other options:
1. gate fullscreen on site engagement
2. gate fullscreen having had a user gesture on the page at any point
3. require a user gesture for each fullscreen request

#3 is the most restrictive. Do we have use cases for going into fullscreen that don't involve a user gesture? I can't think of any at the moment.

> 1. gate fullscreen on site engagement

I don't want to do this. I've seen site engagement suggested as a crutch for getting around a lot of spammy issues. I don't like that direction, since in aggregate it means websites won't be very useful until you interact with them a bunch.

> 2. gate fullscreen having had a user gesture on the page at any point
> 3. require a user gesture for each fullscreen request

Fullscreen already requires a user gesture. The problem is that the hitting Esc to exit *is* a user gesture, so the site is allowed to go fullscreen again.

This gives me some ideas that are probably better than my timing suggestion above:

1. The keyup associated with pressing Esc to exit fullscreen is *not* considered a user gesture (so is not allowed to exit fullscreen or do any other spammy action).
2. We do not deliver a keyup event corresponding to the keypress to exit fullscreen. Since the browser captures the keydown, it should also capture the keyup.

#2 is probably ideal for other reasons anyway (it's weird to deliver a keyup without a corresponding keydown).

I agree that if ESC key press is used to exit an exclusive access (fullscreen, pointer, or keyboard lock) then we should not deliver that input in any form (down, press, up) to web content. 

Lol. Yes, it should definitely not grant a user gesture.

I agree that delivering keyup without keydown is weird, but I'm not sure the current behavior of not delivering keydown is desired.

It makes more sense to me to deliver a down/press/up sequence and ignore the preventDefault call than it does to not deliver them, but I could see it argued either way. For example, I could imagine youtube wanting to track how often people exit fullscreen via the escape key. If we don't deliver the event at all, they have no way to detect that.

mgiuca, it sounds like you're interested in owning this. Mind if we assign it to you?

+dtapuska, since we're talking user gestures and key events.

> It makes more sense to me to deliver a down/press/up sequence and ignore the preventDefault call

I don't see how that will help fix this bug: the issue isn't that the esc is being preventDefault'd. It's that fullscreen mode is being re-requested on keyup, and we allow that because the keyup is a user gesture.

It sounds like you're (ojan) leaning towards #7 suggestion 1 (deliver the events but don't consider it a user gesture for the purpose of deciding what capabilities are allowed).

> I could imagine youtube wanting to track how often people exit fullscreen via the escape key

I don't really see why we should give the site access to those signals. They can already detect when fullscreen mode is exited; do they really need to see when the escape key was used versus the mouse?

It might be worth figuring out whether other browsers deliver this signal.

> mgiuca, it sounds like you're interested in owning this. Mind if we assign it to you?

Yeah OK, I'm not sure I have time to work on this immediately but I should be able to get to it in the M56 timeframe.

Adjusting the labels given that this has been an issue since at least March, it doesn't need to be RBS 55.

I think doing this in time for M56 is ideal. Thanks!

I didn't mean the preventDefault thing as a solution to this problem, but rather as a way of firing the events without allowing the page to prevent esc from exiting fullscreen.

For this issue, I was thinking that not granting a user gesture was the right solution, but I think that depends on whether we fire the keydown event. If we decide not firing keydown is the right thing to do, then I think not firing keyup is also correct (#7 suggestion 2).

But, if we were starting with a clean slate, I would fire all the regular key events, ignore or disallow preventDefault, and not grant a user gesture.

I agree that the next step is to see what other browsers do with the key events.

Can we deliver the key events to the app but mark them as non-cancelable?
And for non-cancelable key events don't generate a user gesture?

I'm sure that's possible (but I don't exactly know how), and would fix the issue. First we should decide if we want to do that, or just not deliver the keyup at all.

 Issue 100441  is a very old issue mentioned in Browser::PreHandleKeyboardEvent. The comment says "This may consume the event (e.g., Esc exits fullscreen mode)."

It looks like the keydown and keypress events are in fact consumed and never delivered, but the keyup event is. In the interest of moving forward, I suggest also consuming the keyup event.

Looks like Firefox consumes the events as well. Let's go with that for now. m56 branches in 10 days. mguica, any can you can prioritize getting this fixed before then?

Yes.

 Issue 641711  has been merged into this issue.

foolip@ has stepped up and mailed out a patch that fixes this:
https://codereview.chromium.org/2482853002/

So assigning to him to finish that up. Thanks Philip!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/35d322e24f91a372ecdc0b152891e0635187a07e

commit 35d322e24f91a372ecdc0b152891e0635187a07e
Author: foolip <foolip@chromium.org>
Date: Wed Nov 09 15:45:28 2016

Suppress KeyUp events after PreHandleKeyboardEvent consumes RawKeyDown

BUG= 654140 

Review-Url: https://codereview.chromium.org/2482853002
Cr-Commit-Position: refs/heads/master@{#430939}

[modify] https://crrev.com/35d322e24f91a372ecdc0b152891e0635187a07e/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/35d322e24f91a372ecdc0b152891e0635187a07e/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/35d322e24f91a372ecdc0b152891e0635187a07e/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/35d322e24f91a372ecdc0b152891e0635187a07e/content/browser/renderer_host/render_widget_host_view_aura.cc

 Issue 142524  has been merged into this issue.

 Issue 672515  has been merged into this issue.

 Issue 678280  has been merged into this issue.

 Issue 680630  has been merged into this issue.

 Issue 682233  has been merged into this issue.

 Issue 749243  has been merged into this issue.