Allow users to perform queries in the omnibox which begin with a search shortcut.

Allow users to perform queries in the omnibox which begin with a search
shortcut.

Previously, if a user had (for example) configured a search shortcut of 'cs' which
went to cs.chromium.org, they would be unable to (easily) start a query in the
omnibox starting with 'cs'. Now, backspacing the keyword leaves the space after
the keyword, allowing the user to continue typing without reenabling keyword
mode.

BUG=393371
NOTRY=true
R=pkasting@chromium.org
TEST=type 'google.com ' into the omnibar and it should enter keyword mode.
Press backspace and 'google.com ' will be left in the omnibar, rather than
'google.com'.

Committed: https://crrev.com/709649607f13acb04e3850d5fd7fc1903c1becae
Cr-Commit-Position: refs/heads/master@{#322429}

[khzn, 2014-08-15 12:54:59]

The CQ bit was checked by lenny.khazan@gmail.com

[khzn, 2014-08-15 12:55:01]

The CQ bit was unchecked by lenny.khazan@gmail.com

[sky, 2014-08-15 18:10:29]

Peter should review this. He'll no doubt have opinions about the correctness of
this as well as any subtleties this may cause.

https://codereview.chromium.org/477873002/diff/1/chrome/browser/ui/omnibox/om...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

https://codereview.chromium.org/477873002/diff/1/chrome/browser/ui/omnibox/om...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:904:
reinterpret_cast<base::char16 const*>(L" ") + visible_text);
This isn't the right way to get a string16. You want ASCIIToUTF16(" ").

[Peter Kasting, 2014-08-22 00:41:13]

I'm not at work right now so I can't check; did you sign the Committer License
Agreement?

LGTM

https://codereview.chromium.org/477873002/diff/20001/chrome/browser/ui/omnibo...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

https://codereview.chromium.org/477873002/diff/20001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:904: // re-enabling keyword mode
(http://crbug.com/393371)
Nit: Don't refer to bugs in code comments.  Provide any necessary explanation
for the reader directly in the comment.

https://codereview.chromium.org/477873002/diff/20001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:905: const base::string16
window_text(keyword_ + base::ASCIIToUTF16(" ") +
Nit: Break after initial '(' instead; consider using assignment form instead of
constructor form to do the initialization to make this wrapping less
strange-looking

https://codereview.chromium.org/477873002/diff/20001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:906: visible_text);
Nit: Continuations of previous lines are indented 4, not 2

[khzn, 2014-08-22 01:42:58]

On 2014/08/22 01:42:36, khzn wrote:

Yes, I signed the Committer License Agreement.

[Peter Kasting, 2015-02-20 01:39:15]

Sorry, somehow this got left in limbo!  Please feel free to bug me when I don't
take action.

I checked the CLA signatories and have at least one, possibly two, entries for
you, but neither matches the email you put in the AUTHORS file.  Can you either
change the AUTHORS email to match the signature on file, or else re-sign the CLA
with this e-mail address?

https://codereview.chromium.org/477873002/diff/40001/chrome/browser/ui/omnibo...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

https://codereview.chromium.org/477873002/diff/40001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:904: // re-enabling keyword mode
Nit: Trailing period

[khzn, 2015-03-26 18:20:09]

On 2015/02/20 01:39:15, Peter Kasting wrote:
> Sorry, somehow this got left in limbo!  Please feel free to bug me when I
don't
> take action.
> 
> I checked the CLA signatories and have at least one, possibly two, entries for
> you, but neither matches the email you put in the AUTHORS file.  Can you
either
> change the AUTHORS email to match the signature on file, or else re-sign the
CLA
> with this e-mail address?
> 
>
https://codereview.chromium.org/477873002/diff/40001/chrome/browser/ui/omnibo...
> File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):
> 
>
https://codereview.chromium.org/477873002/diff/40001/chrome/browser/ui/omnibo...
> chrome/browser/ui/omnibox/omnibox_edit_model.cc:904: // re-enabling keyword
mode
> Nit: Trailing period

Apologies, I re-signed the CLA with the email address in the AUTHORS file.

[khzn, 2015-03-26 18:21:03]

The CQ bit was checked by lenny.khazan@gmail.com

[commit-bot: I haz the power, 2015-03-26 18:22:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/477873002/60001

[commit-bot: I haz the power, 2015-03-26 18:23:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-26 18:24:00]

Patchset 4 (id:??) landed as
https://crrev.com/709649607f13acb04e3850d5fd7fc1903c1becae
Cr-Commit-Position: refs/heads/master@{#322429}

[scheib, 2015-03-26 19:30:09]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1030413002/ by scheib@chromium.org.

The reason for reverting is: Caused a few OmniboxViewTest interactive_ui_tests
failures (tests need updating too). 

build:
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/22466

test output:
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...

Looks like try jobs weren't run, only commit queue earlier. Unsure why commit
queue didn't include this test - but it's recommended to run 'git cl try' on
patches and review..

[Peter Kasting, 2015-03-26 20:14:42]

Lenny, can you reupload this patch as a new CL and then fix the test failures
above?

[khzn, 2015-03-26 20:27:46]

Sure thing - how can I create a new CL from the same local branch?

[Peter Kasting, 2015-03-26 20:28:32]

On 2015/03/26 20:27:46, khzn wrote:
> Sure thing - how can I create a new CL from the same local branch?

I wouldn't -- I'd use the "Download [raw]" link above to grab the patch, then
apply it to a new branch.