Issue metadata
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 descriptionUserAgent: 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.
,
Oct 7 2016
Issue 653497 has been merged into this issue.
,
Oct 7 2016
: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.
,
Oct 10 2016
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.
,
Oct 10 2016
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.
,
Oct 10 2016
,
Nov 3 2016
,
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
,
Nov 14 2016
The fix is now on Canary. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by tapted@chromium.org
, Oct 7 2016Owner: dominickn@chromium.org
Status: Assigned (was: Unconfirmed)