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

Issue 7754008: Omnibox enters keyword search mode incorrectly (Closed)

Created:
9 years, 3 months ago by keishi
Modified:
9 years, 2 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Omnibox enters keyword search mode incorrectly Happens when input starts with default search engine keyword. BUG=95454 TEST=1. Set default search engine keyword to 'g'. 2. Type 'grand canyon' into omnibox. 3. Confirm that it is not in keyword search mode. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102688

Patch Set 1 #

Total comments: 1

Patch Set 2 : better patch #

Patch Set 3 : Even better patch #

Patch Set 4 : added test #

Total comments: 16

Patch Set 5 : fixed nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+125 lines, -21 lines) Patch
M chrome/browser/autocomplete/autocomplete_popup_model.cc View 1 2 3 1 chunk +6 lines, -21 lines 0 comments Download
A chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc View 1 2 3 4 1 chunk +118 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
keishi
9 years, 3 months ago (2011-09-08 03:18:21 UTC) #1
sky
http://codereview.chromium.org/7754008/diff/1/chrome/browser/autocomplete/autocomplete_popup_model.cc File chrome/browser/autocomplete/autocomplete_popup_model.cc (right): http://codereview.chromium.org/7754008/diff/1/chrome/browser/autocomplete/autocomplete_popup_model.cc#newcode145 chrome/browser/autocomplete/autocomplete_popup_model.cc:145: const string16& input_text = autocomplete_controller()->input().text(); This shouldn't be necessary. ...
9 years, 3 months ago (2011-09-08 14:21:49 UTC) #2
keishi
Yes I am able to reproduce it. I get SEARCH_HISTORY as my match.type So I ...
9 years, 3 months ago (2011-09-08 18:33:26 UTC) #3
sky
With this change, what happens when you press space? I think this bug is a ...
9 years, 3 months ago (2011-09-09 03:44:51 UTC) #4
Peter Kasting
For patch set 2: Does this still work correctly if you type the default engine's ...
9 years, 3 months ago (2011-09-12 18:16:09 UTC) #5
keishi
On 2011/09/12 18:16:09, Peter Kasting wrote: > For patch set 2: Does this still work ...
9 years, 3 months ago (2011-09-13 04:46:41 UTC) #6
sky
Can you add a test case for coverage? -Scott
9 years, 3 months ago (2011-09-13 17:25:11 UTC) #7
keishi
On 2011/09/13 17:25:11, sky wrote: > Can you add a test case for coverage? > ...
9 years, 3 months ago (2011-09-14 10:20:36 UTC) #8
Peter Kasting
LGTM http://codereview.chromium.org/7754008/diff/14001/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc File chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc (right): http://codereview.chromium.org/7754008/diff/14001/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc#newcode20 chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc:20: // See description above class for what this ...
9 years, 3 months ago (2011-09-14 17:49:37 UTC) #9
keishi
http://codereview.chromium.org/7754008/diff/14001/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc File chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc (right): http://codereview.chromium.org/7754008/diff/14001/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc#newcode20 chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc:20: // See description above class for what this registers. ...
9 years, 3 months ago (2011-09-15 02:23:12 UTC) #10
commit-bot: I haz the power
CQ is trying tha patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/7754008/10007
9 years, 2 months ago (2011-09-26 01:35:00 UTC) #11
commit-bot: I haz the power
9 years, 2 months ago (2011-09-26 02:45:55 UTC) #12
Change committed as 102688

Powered by Google App Engine
This is Rietveld 408576698