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

Issue 399032: Fix a crash when activating a select element inside a page action (Closed)

Created:
11 years, 1 month ago by Aaron Boodman
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Fix a crash when activating a select element inside a page action popup. With this change, select elements still don't work correctly with page actions: when you try to use them, the page action popup disappears. However, at least now, it doesn't crash. BUG=27576 TEST=Install extension in related bug. Navigate to any site and click page action. Browser should not crash. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=32277

Patch Set 1 #

Patch Set 2 : cleanup #

Total comments: 1

Patch Set 3 : Only allow one popup at a time #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -12 lines) Patch
M chrome/browser/views/browser_bubble.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/views/location_bar_view.h View 1 6 chunks +17 lines, -1 line 0 comments Download
M chrome/browser/views/location_bar_view.cc View 1 2 4 chunks +50 lines, -9 lines 1 comment Download

Messages

Total messages: 4 (0 generated)
Aaron Boodman
11 years, 1 month ago (2009-11-17 07:29:00 UTC) #1
Erik does not do reviews
http://codereview.chromium.org/399032/diff/1001/9 File chrome/browser/views/location_bar_view.cc (right): http://codereview.chromium.org/399032/diff/1001/9#newcode1344 Line 1344: popup_ = ExtensionPopup::Show(page_action_->popup_url(), browser, rect, Given the dismiss ...
11 years, 1 month ago (2009-11-17 16:32:17 UTC) #2
Aaron Boodman
Good point, fixed that, and moved the coordinate calculation down closer to where it is ...
11 years, 1 month ago (2009-11-18 00:50:40 UTC) #3
Erik does not do reviews
11 years, 1 month ago (2009-11-18 00:55:18 UTC) #4
LGTM

http://codereview.chromium.org/399032/diff/3002/5002
File chrome/browser/views/location_bar_view.cc (right):

http://codereview.chromium.org/399032/diff/3002/5002#newcode1339
Line 1339: HidePopup();
to avoid flicker, you could also check if the current popup is the same as the
new one, but that could be a TODO for now.

Powered by Google App Engine
This is Rietveld 408576698