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

Unified Diff: chrome/browser/ui/search/instant_controller.cc

Issue 15001020: InstantExtended: disallow setValue() without omnibox focus. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix typo. Created 7 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/search/instant_controller.cc
diff --git a/chrome/browser/ui/search/instant_controller.cc b/chrome/browser/ui/search/instant_controller.cc
index e6e9bdd305e8cc1066c573e052653baed6c1c59a..1885b1f9c69311b2d7dad18b291789e02e3a6c8d 100644
--- a/chrome/browser/ui/search/instant_controller.cc
+++ b/chrome/browser/ui/search/instant_controller.cc
@@ -1334,6 +1334,21 @@ void InstantController::SetSuggestions(
return;
if (suggestion.behavior == INSTANT_COMPLETE_REPLACE) {
+ if (omnibox_focus_state_ == OMNIBOX_FOCUS_NONE) {
+ // TODO(samarth,skanuj): setValue() needs to be handled differently when
+ // the omnibox doesn't have focus. Instead of setting temporary text, we
+ // should be setting search terms on the appropriate NavigationEntry.
+ // (Among other things, this ensures that URL-shaped values will get the
+ // additional security token.)
+ //
+ // Note that this also breaks clicking on a suggestion corresponding to
+ // gray-text completion: we can't distinguish between the user
+ // clicking on white space (where we don't accept the gray text) and the
+ // user clicking on the suggestion (when we do accept the gray text).
+ // This needs to be fixed before we can turn on Instant again.
+ return;
+ }
+
// We don't get an Update() when changing the omnibox due to a REPLACE
// suggestion (so that we don't inadvertently cause the overlay to change
// what it's showing, as the user arrows up/down through the page-provided
@@ -1419,7 +1434,9 @@ void InstantController::FocusOmnibox(const content::WebContents* contents,
// doing nothing instead of crashing the browser process (intentional no-op).
switch (state) {
case OMNIBOX_FOCUS_VISIBLE:
- browser_->FocusOmnibox(true);
+ // TODO(samarth): re-enable this once setValue() correctly handles
+ // URL-shaped queries.
+ // browser_->FocusOmnibox(true);
break;
case OMNIBOX_FOCUS_INVISIBLE:
browser_->FocusOmnibox(false);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698