Commit 2ff3ed41 authored by samarth@chromium.org's avatar samarth@chromium.org

InstantExtended: disallow setValue() without omnibox focus.

Don't allow the the page to call setValue() if the omnibox doesn't have focus.
This breaks the "clearing out gray text" usecase (when the user clicks on
the suggestion corresponding to a gray-text completion), but we aren't
worried about it because we are aiming for a launch without any gray text.
We'll re-enable setValue() in the general case when we handle it properly.

Also disable the focus() API to prevent a trivial workaround (where the page
focuses the omnibox and then calls setValue).

BUG=229193

Review URL: https://chromiumcodereview.appspot.com/15001020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203035 0039d316-1c4b-4281-b951-d872f2087c98
parent 267a1d2e
...@@ -1322,6 +1322,21 @@ void InstantController::SetSuggestions( ...@@ -1322,6 +1322,21 @@ void InstantController::SetSuggestions(
return; return;
if (suggestion.behavior == INSTANT_COMPLETE_REPLACE) { 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 // 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 // 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 // what it's showing, as the user arrows up/down through the page-provided
...@@ -1407,7 +1422,9 @@ void InstantController::FocusOmnibox(const content::WebContents* contents, ...@@ -1407,7 +1422,9 @@ void InstantController::FocusOmnibox(const content::WebContents* contents,
// doing nothing instead of crashing the browser process (intentional no-op). // doing nothing instead of crashing the browser process (intentional no-op).
switch (state) { switch (state) {
case OMNIBOX_FOCUS_VISIBLE: case OMNIBOX_FOCUS_VISIBLE:
browser_->FocusOmnibox(true); // TODO(samarth): re-enable this once setValue() correctly handles
// URL-shaped queries.
// browser_->FocusOmnibox(true);
break; break;
case OMNIBOX_FOCUS_INVISIBLE: case OMNIBOX_FOCUS_INVISIBLE:
browser_->FocusOmnibox(false); browser_->FocusOmnibox(false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment