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

Issue 402036: Fixes several focus issue with extension browser action popups (Closed)

Created:
11 years, 1 month ago by jcampan
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com, Aaron Boodman, Erik does not do reviews, pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Fixes several focus issue with popup in the extensions' browser actions: - when opening a browser action popup, it gets the focus. - tab traversal now works in the popup - pressing esc closes the popup (if the keyboard event is not processed by the page) BUG=22654, 28087, 28086 TEST=Create an extension with a browser extension that shows a popup. Make the popup so that it has a textfield (that gets focused when the popup show) and a button. Install the extension. Open the popup, the textfield should have focus. Press tab, the focus should go to the button. Press Esc, the popup should be closed. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=32335

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 4

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -2 lines) Patch
M chrome/browser/extensions/extension_host.cc View 1 2 3 3 chunks +17 lines, -0 lines 0 comments Download
M chrome/browser/views/browser_actions_container.cc View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/views/browser_bubble_win.cc View 1 chunk +6 lines, -2 lines 0 comments Download
M chrome/browser/views/extensions/extension_view.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/views/extensions/extension_view.cc View 1 2 3 2 chunks +11 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
jcampan
11 years, 1 month ago (2009-11-18 01:48:39 UTC) #1
Erik does not do reviews
11 years, 1 month ago (2009-11-18 01:59:35 UTC) #2
LGTM - just nits

http://codereview.chromium.org/402036/diff/9/1015
File chrome/browser/extensions/extension_host.cc (right):

http://codereview.chromium.org/402036/diff/9/1015#newcode485
Line 485: #endif
add TODO(port)

http://codereview.chromium.org/402036/diff/9/1019
File chrome/browser/views/browser_actions_container.cc (right):

http://codereview.chromium.org/402036/diff/9/1019#newcode457
Line 457: // Forward the focus to the renderer.
nit: newline above comment

http://codereview.chromium.org/402036/diff/9/1017
File chrome/browser/views/extensions/extension_view.cc (right):

http://codereview.chromium.org/402036/diff/9/1017#newcode25
Line 25: // This view needs to be focusable so it can act as the focused view
for the
nit: newline above comment.

http://codereview.chromium.org/402036/diff/9/1017#newcode181
Line 181: 
nit: remove newline

Powered by Google App Engine
This is Rietveld 408576698