New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 653498 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug
Team-Security-UX



Sign in to add a comment

Press ESC twice prevents camera dialog from showing again

Reported by joe.pal...@iproov.com, Oct 6 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36

Steps to reproduce the problem:
1. https://webrtc.github.io/samples/src/content/getusermedia/gum/
2. Press ESC twice (or more)
3. Refresh the page

What is the expected behavior?
The camera permission dialog box should show again.

What went wrong?
Nothing showed

Did this work before? N/A 

Chrome version: 53.0.2785.143  Channel: stable
OS Version: OS X 10.11.6
Flash Version: Shockwave Flash 23.0 r0

If you switch to another tab and then back, the dialog box is then showing.
 
Components: -UI UI>Browser>Permissions
Owner: dominickn@chromium.org
Status: Assigned (was: Unconfirmed)
[mac triage] Dom - wanna take a look? This is bizarre.

Happens for any permission bubble. E.g.

- https://permission.site/
- Click Notifications [request shows]
- Esc ONCE dismisses request, reload, click Notifications, requests shows OK
- Esc TWICE also dismisses request. But then reload, Notifications, request does NOT show
 Issue 653497  has been merged into this issue.
Cc: kcaratt...@chromium.org raymes@chromium.org
:S

Can definitely confirm the behaviour, and that switching tabs gets the bubble back.

The bug also exists in the current Mac Canary. Will look into debugging on Monday. +cc permissions folks.
What's going on is that the first ESC happens while the permission prompt has focus, so the prompt takes the event.

The second ESC happens with the browser window focused, triggering BrowserWindowCoca::PreHandleKeyboardEvent to call BrowserWindowController::dismissPermissionBubble, even though there isn't a permission bubble visible. That destroys the permission prompt impl object held in PermissionManager::view_, so when the page is refreshed and the new permission request comes in, the isn't a prompt impl to display and nothing happens.

This is special on Mac because on that platform we seem to need to explicitly handle an ESC while permission bubbles aren't focused (on Windows it seems to just happen) See crbug.com/385088.

The fix is to make BrowserWindowController::dismissPermissionBubble check whether there are any permission bubbles, rather than unconditionally hiding them on ESC.
It turns out the proposed fix in #4 requires more nuance. This bug manifests itself as well when you trigger a permission prompt, click on the page somewhere to take focus away from the bubble, and then use ESC to dismiss the bubble. The proposed fix would let this case through and cause the same missing prompt problem.

Instead, I'm going to forcibly recreate the view if it's been deleted in PermissionRequestManager::FinalizeBubble. That method already tries to display pending requests, so if the view is gone then it should be recreated. There will be some associated fixes to ensure that Mac bubbles correctly called Closing() so that dismissals are treated correctly too.
Cc: f...@chromium.org groby@chromium.org
Components: -UI>Browser>Permissions UI>Browser>Permissions>Prompts
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 10 2016

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

commit 3fe468bfc1f6180bacebb8fcc0cbf9effc9394ab
Author: dominickn <dominickn@chromium.org>
Date: Thu Nov 10 00:28:23 2016

[Mac] Address buggy permission bubble behaviour on dismissal via ESC.

On Mac, triggering a permission prompt, dismissing it by pressing ESC
*twice*, then refreshing the page and triggering the prompt again
results in no prompt being displayed until the tab is switched away and
back. Pressing ESC *once* works as expected.

This happens because pressing the ESC key with the browser window
focused results in the PermissionRequestManager hiding all permission
bubbles, which has the side-effect of destroying the prompt UI surface.
The next prompt triggered then has no surface to be displayed on,
delaying its display until DisplayPendingRequests is called and the
surface is recreated (e.g. when switching tabs).

This CL fixes the problem by making the PermissionRequestManager
dismiss any open permission bubble, rather than hide them. This does not
destroy the prompt UI surface.

Additionally, pressing ESC on Mac does not properly dismiss a permission
prompt, i.e. the PermissionDecided callback is *not* run when a bubble
is dismissed with the keyboard, and the site is not informed of the
denied permission. This CL also addresses that issue by overriding
PermissionBubbleController::cancel to call
PermissionRequestManager::Closing(), and adding a new method called by the
Mac BrowserWindowController that also calls Closing(). This ensures that
old requests are always cleared when the bubble is hidden (otherwise they
will remain in the queue).

BUG=385088,  653497 ,  653498 
TEST=Visit a page which displays a permission bubble. Dismiss the bubble
by pressing ESC. Ensure that the permission denied callback is run.
Press ESC a second time. Refresh the page and ensure the permission
prompt can be seen.

TEST=Visit a page which displays a permission bubble. Click on the page
(not on the bubble) to change focus. Press ESC and ensure the bubble is
dismissed. Ensure that the permission denied callback is run. Refresh
the page and ensure the permission prompt can be seen.

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

[modify] https://crrev.com/3fe468bfc1f6180bacebb8fcc0cbf9effc9394ab/chrome/browser/permissions/permission_request_manager.h
[modify] https://crrev.com/3fe468bfc1f6180bacebb8fcc0cbf9effc9394ab/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/3fe468bfc1f6180bacebb8fcc0cbf9effc9394ab/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm
[modify] https://crrev.com/3fe468bfc1f6180bacebb8fcc0cbf9effc9394ab/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
[modify] https://crrev.com/3fe468bfc1f6180bacebb8fcc0cbf9effc9394ab/chrome/browser/ui/views/website_settings/permission_prompt_impl.cc

Status: Fixed (was: Assigned)
The fix is now on Canary.

Sign in to add a comment