Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(8)

Issue 1137543004: [Permission Bubble] Allow re-display after ESC (Closed)

Created:
5 years, 7 months ago by groby-ooo-7-16
Modified:
5 years, 7 months ago
Reviewers:
felt
CC:
chromium-reviews, hcarmona
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Permission Bubble] Allow re-display after ESC Permission bubbles on Mac would not re-display for new permissions after dismissing a bubble with ESC. The root cause is that ESC would close the bubble, but not clean out the pending requests. This change ensures pending requests are removed when a bubble closes, for whatever reason. BUG=385088 TEST= 1) Navigate to adrifelt.github.io/demos/all-permissions.html 2) Click on location, observer permission bubble popping up. 3) Click in permission bubble to give focus to bubble - DO NOT click on any UI elements in the bubble, just the text. 4) Press ESC, observe bubble being dismissed. 5) Click on "notifications" on the web page. With the change, observe a new bubble. Without, no new bubble. Committed: https://crrev.com/3f2fe29dce19b9ed2479313fd6ee631a160a23f6 Cr-Commit-Position: refs/heads/master@{#330950}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/website_settings/permission_bubble_controller_unittest.mm View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 10 (2 generated)
groby-ooo-7-16
PTAL
5 years, 7 months ago (2015-05-20 19:56:16 UTC) #2
felt
On 2015/05/20 19:56:16, groby wrote: > PTAL Is this what made you notice the issue ...
5 years, 7 months ago (2015-05-21 17:02:51 UTC) #3
felt
lgtm, good find
5 years, 7 months ago (2015-05-21 17:03:21 UTC) #4
groby-ooo-7-16
You'll love to hear that there might be other loopholes where the bubble might get ...
5 years, 7 months ago (2015-05-21 17:06:54 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1137543004/1
5 years, 7 months ago (2015-05-21 17:08:09 UTC) #7
felt
On 2015/05/21 17:06:54, groby wrote: > You'll love to hear that there might be other ...
5 years, 7 months ago (2015-05-21 17:11:46 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 7 months ago (2015-05-21 17:59:18 UTC) #9
commit-bot: I haz the power
5 years, 7 months ago (2015-05-21 18:00:08 UTC) #10
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/3f2fe29dce19b9ed2479313fd6ee631a160a23f6
Cr-Commit-Position: refs/heads/master@{#330950}

Powered by Google App Engine
This is Rietveld 408576698