diff options
9 files changed, 111 insertions, 28 deletions
diff --git a/chrome/browser/extensions/extension_popup_api.cc b/chrome/browser/extensions/extension_popup_api.cc index f778e0e..091fa71 100644 --- a/chrome/browser/extensions/extension_popup_api.cc +++ b/chrome/browser/extensions/extension_popup_api.cc @@ -42,6 +42,8 @@ const wchar_t kWidthKey[] = L"width"; const wchar_t kHeightKey[] = L"height"; const wchar_t kTopKey[] = L"top"; const wchar_t kLeftKey[] = L"left"; +const wchar_t kGiveFocusKey[] = L"giveFocus"; +const wchar_t kDomAnchorKey[] = L"domAnchor"; }; // namespace @@ -81,8 +83,12 @@ bool PopupShowFunction::RunImpl() { std::string url_string; EXTENSION_FUNCTION_VALIDATE(args->GetString(0, &url_string)); + DictionaryValue* show_details = NULL; + EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &show_details)); + DictionaryValue* dom_anchor = NULL; - EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &dom_anchor)); + EXTENSION_FUNCTION_VALIDATE(show_details->GetDictionary(kDomAnchorKey, + &dom_anchor)); int dom_top, dom_left; EXTENSION_FUNCTION_VALIDATE(dom_anchor->GetInteger(kTopKey, @@ -98,6 +104,13 @@ bool PopupShowFunction::RunImpl() { EXTENSION_FUNCTION_VALIDATE(dom_top >= 0 && dom_left >= 0 && dom_width >= 0 && dom_height >= 0); + // The default behaviour is to give the focus to the pop-up window. + bool give_focus = true; + if (show_details->HasKey(kGiveFocusKey)) { + EXTENSION_FUNCTION_VALIDATE(show_details->GetBoolean(kGiveFocusKey, + &give_focus)); + } + GURL url = dispatcher()->url().Resolve(url_string); if (!url.is_valid()) { error_ = kInvalidURLError; @@ -127,7 +140,7 @@ bool PopupShowFunction::RunImpl() { (NULL != dispatcher()->GetExtensionHost()) ? BubbleBorder::BOTTOM_LEFT : BubbleBorder::TOP_LEFT; popup_ = ExtensionPopup::Show(url, dispatcher()->GetBrowser(), rect, - arrow_location); + arrow_location, give_focus); ExtensionPopupHost* popup_host = dispatcher()->GetPopupHost(); DCHECK(popup_host); diff --git a/chrome/browser/extensions/extension_popup_apitest.cc b/chrome/browser/extensions/extension_popup_apitest.cc index d8e33af..16c5374 100644 --- a/chrome/browser/extensions/extension_popup_apitest.cc +++ b/chrome/browser/extensions/extension_popup_apitest.cc @@ -10,5 +10,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, FLAKY_Popup) { CommandLine::ForCurrentProcess()->AppendSwitch( switches::kEnableExperimentalExtensionApis); + CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableExtensionToolstrips); ASSERT_TRUE(RunExtensionTest("popup_api")) << message_; } diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc index 2ca49a9..999f3d3 100644 --- a/chrome/browser/views/browser_actions_container.cc +++ b/chrome/browser/views/browser_actions_container.cc @@ -379,7 +379,8 @@ void BrowserActionsContainer::OnBrowserActionExecuted( popup_ = ExtensionPopup::Show(browser_action->popup_url(), toolbar_->browser(), rect, - BubbleBorder::TOP_RIGHT); + BubbleBorder::TOP_RIGHT, + true); // Activate the popup window. popup_->set_delegate(this); popup_button_ = button; popup_button_->PopupDidShow(); diff --git a/chrome/browser/views/extensions/extension_popup.cc b/chrome/browser/views/extensions/extension_popup.cc index 0be410c..c9c7ae6 100644 --- a/chrome/browser/views/extensions/extension_popup.cc +++ b/chrome/browser/views/extensions/extension_popup.cc @@ -29,12 +29,14 @@ const int ExtensionPopup::kMaxHeight = 600; ExtensionPopup::ExtensionPopup(ExtensionHost* host, Widget* frame, const gfx::Rect& relative_to, - BubbleBorder::ArrowLocation arrow_location) + BubbleBorder::ArrowLocation arrow_location, + bool activate_on_show) : BrowserBubble(host->view(), frame, gfx::Point()), relative_to_(relative_to), - extension_host_(host) { + extension_host_(host), + activate_on_show_(activate_on_show) { host->view()->SetContainer(this); registrar_.Add(this, NotificationType::EXTENSION_HOST_DID_STOP_LOADING, @@ -56,6 +58,10 @@ ExtensionPopup::ExtensionPopup(ExtensionHost* host, border_view_->set_background(new BubbleBackground(border_)); border_view_->set_border(border_); border_widget_->SetContentsView(border_view_); + + // Ensure that the popup contents are always displayed ontop of the border + // widget. + border_widget_->MoveAbove(popup_); } ExtensionPopup::~ExtensionPopup() { @@ -68,7 +74,7 @@ void ExtensionPopup::Hide() { border_widget_->Hide(); } -void ExtensionPopup::Show() { +void ExtensionPopup::Show(bool activate) { if (visible()) return; @@ -80,7 +86,7 @@ void ExtensionPopup::Show() { // Show the border first, then the popup overlaid on top. border_widget_->Show(); - BrowserBubble::Show(true); + BrowserBubble::Show(activate); } void ExtensionPopup::ResizeToView() { @@ -118,7 +124,7 @@ void ExtensionPopup::Observe(NotificationType type, // Once we receive did stop loading, the content will be complete and // the width will have been computed. Now it's safe to show. if (extension_host_.get() == Details<ExtensionHost>(details).ptr()) - Show(); + Show(activate_on_show_); } else { NOTREACHED() << L"Received unexpected notification"; } @@ -138,7 +144,8 @@ void ExtensionPopup::OnExtensionPreferredSizeChanged(ExtensionView* view) { ExtensionPopup* ExtensionPopup::Show( const GURL& url, Browser* browser, const gfx::Rect& relative_to, - BubbleBorder::ArrowLocation arrow_location) { + BubbleBorder::ArrowLocation arrow_location, + bool activate_on_show) { ExtensionProcessManager* manager = browser->profile()->GetExtensionProcessManager(); DCHECK(manager); @@ -149,12 +156,12 @@ ExtensionPopup* ExtensionPopup::Show( views::Widget* frame = BrowserView::GetBrowserViewForNativeWindow( browser->window()->GetNativeHandle())->GetWidget(); ExtensionPopup* popup = new ExtensionPopup(host, frame, relative_to, - arrow_location); + arrow_location, activate_on_show); // If the host had somehow finished loading, then we'd miss the notification // and not show. This seems to happen in single-process mode. if (host->did_stop_loading()) - popup->Show(); + popup->Show(activate_on_show); return popup; } diff --git a/chrome/browser/views/extensions/extension_popup.h b/chrome/browser/views/extensions/extension_popup.h index 7db472a..4c3ca11 100644 --- a/chrome/browser/views/extensions/extension_popup.h +++ b/chrome/browser/views/extensions/extension_popup.h @@ -29,18 +29,20 @@ class ExtensionPopup : public BrowserBubble, // by value of |arrow_location| remains fixed during popup resizes. // If |arrow_location| is BOTTOM_*, then the popup 'pops up', otherwise // the popup 'drops down'. + // Pass |activate_on_show| as true to activate the popup window. // // The actual display of the popup is delayed until the page contents // finish loading in order to minimize UI flashing and resizing. static ExtensionPopup* Show(const GURL& url, Browser* browser, const gfx::Rect& relative_to, - BubbleBorder::ArrowLocation arrow_location); + BubbleBorder::ArrowLocation arrow_location, + bool activate_on_show); ExtensionHost* host() const { return extension_host_.get(); } // BrowserBubble overrides. virtual void Hide(); - virtual void Show(); + virtual void Show(bool activate); virtual void ResizeToView(); // NotificationObserver overrides. @@ -63,7 +65,8 @@ class ExtensionPopup : public BrowserBubble, ExtensionPopup(ExtensionHost* host, views::Widget* frame, const gfx::Rect& relative_to, - BubbleBorder::ArrowLocation); + BubbleBorder::ArrowLocation arrow_location, + bool activate_on_show); // The area on the screen that the popup should be positioned relative to. gfx::Rect relative_to_; @@ -71,6 +74,9 @@ class ExtensionPopup : public BrowserBubble, // The contained host for the view. scoped_ptr<ExtensionHost> extension_host_; + // Flag used to indicate if the pop-up should be activated upon first display. + bool activate_on_show_; + NotificationRegistrar registrar_; // A separate widget and associated pieces to draw a border behind the diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc index b0b9069..03f8437 100644 --- a/chrome/browser/views/location_bar_view.cc +++ b/chrome/browser/views/location_bar_view.cc @@ -1351,8 +1351,11 @@ void LocationBarView::PageActionImageView::ExecuteAction(int button) { gfx::Rect rect = parent->bounds(); rect.set_x(origin.x()); rect.set_y(origin.y()); - popup_ = ExtensionPopup::Show(page_action_->popup_url(), browser, rect, - BubbleBorder::TOP_RIGHT); + popup_ = ExtensionPopup::Show(page_action_->popup_url(), + browser, + rect, + BubbleBorder::TOP_RIGHT, + true); // Activate the popup window. popup_->set_delegate(this); } else { ExtensionBrowserEventRouter::GetInstance()->PageActionExecuted( diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json index 4765076..14a7967 100755 --- a/chrome/common/extensions/api/extension_api.json +++ b/chrome/common/extensions/api/extension_api.json @@ -1668,6 +1668,11 @@ "additionalProperties": { "type": "any" }, "isInstanceOf": "HTMLElement", "description": "A HTML DOM object to which the pop-up's position will be made relative." + }, + "giveFocus": { + "type": "boolean", + "description": "Pass true to give the focus to the popup window. The default behaviour is true.", + "optional": true } } }, diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js index 2fdaad7..7d8cac8 100644 --- a/chrome/renderer/resources/extension_process_bindings.js +++ b/chrome/renderer/resources/extension_process_bindings.js @@ -425,19 +425,31 @@ var chrome = chrome || {}; this.definition.parameters[0], { type: "object", - name: "domAnchor", + name: "showDetails", properties: { - top: { type: "integer", minimum: 0 }, - left: { type: "integer", minimum: 0 }, - width: { type: "integer", minimum: 0 }, - height: { type: "integer", minimum: 0 } + domAnchor: { + type: "object", + properties: { + top: { type: "integer", minimum: 0 }, + left: { type: "integer", minimum: 0 }, + width: { type: "integer", minimum: 0 }, + height: { type: "integer", minimum: 0 } + } + }, + giveFocus: { + type: "boolean", + optional: true + } } }, this.definition.parameters[2] ]; return sendRequest(this.name, [url, - getAbsoluteRect(showDetails.relativeTo), + { + domAnchor: getAbsoluteRect(showDetails.relativeTo), + giveFocus: showDetails.giveFocus + }, callback], internalSchema); } diff --git a/chrome/test/data/extensions/api_test/popup_api/toolband.html b/chrome/test/data/extensions/api_test/popup_api/toolband.html index cde3a31..77467ee 100644 --- a/chrome/test/data/extensions/api_test/popup_api/toolband.html +++ b/chrome/test/data/extensions/api_test/popup_api/toolband.html @@ -3,8 +3,41 @@ <script> var globalValue = "I am not 42."; +// Some helper functions that track the focus state of a form on the toolbar. +var formFocused = false; +function onFormFocused() { + formFocused = true; +} + +function onFormBlurred() { + formFocused = false; +} + window.onload = function() { chrome.test.runTests([ + function showNoFocusShift() { + var entryForm = document.getElementById("entryForm").focus(); + chrome.test.assertTrue(formFocused); + + // Validate that displaying a pop-up with the giveFocus parameter assigned + // to false does not touch the focus setting of the input field. + var showDetails = { + "relativeTo": document.getElementById("anchorHere"), + "giveFocus": false + }; + + // The focus should also remain untouched during closing of the popup. + chrome.test.listenOnce(chrome.experimental.popup.onClosed, function(){ + chrome.test.assertTrue(formFocused); + }); + + chrome.experimental.popup.show("toolband_popup.html", + showDetails, + chrome.test.callbackPass(function() { + chrome.test.assertTrue(formFocused); + chrome.experimental.extension.getPopupView().close(); + })); + }, function noPopup() { chrome.test.assertTrue( undefined === chrome.experimental.extension.getPopupView(), @@ -14,7 +47,7 @@ window.onload = function() { function noParentWindow() { chrome.test.assertTrue( undefined === chrome.experimental.popup.getParentWindow(), - "Parent ); + "Parent window accessible outside of popup view."); chrome.test.succeed(); }, function show() { @@ -39,7 +72,6 @@ window.onload = function() { chrome.test.assertEq(42, popupView.theAnswer()); chrome.test.succeed(); }, - function accessHost() { var popupView = chrome.experimental.extension.getPopupView(); chrome.test.assertTrue(popupView != undefined, @@ -52,13 +84,12 @@ window.onload = function() { chrome.test.assertEq(42, globalValue); chrome.test.succeed(); }, - function closePopup() { chrome.test.listenOnce(chrome.experimental.popup.onClosed, function(){ // TODO(twiz): getPopupView() should return undefined, but, due to - // shut-down races, it is sometimes still defined. See BUG - //chrome.test.assertTrue( - // chrome.experimental.extension.getPopupView() == undefined); + // shut-down races, it is sometimes still defined. See BUG 27658. + // chrome.test.assertTrue( + // chrome.experimental.extension.getPopupView() == undefined); }); chrome.experimental.extension.getPopupView().close(); } @@ -70,5 +101,8 @@ window.onload = function() { <div> <span id="anchorHere">TEST</span> </div> +<form> +<input id="entryForm" onfocus="onFormFocused();" onblur="onFormBlurred();"/> +</form> </body> </html> |