Don't reset the omnibox state on command-return.

Don't reset the omnibox state on command-return.

This was originally added in https://codereview.chromium.org/260008/,
motivated it appears by a desire to match Safari's behavior. Safari no
longer does this (the address field state is preserved on command-return
as the new tab is openeed in the background).

This change makes Chrome's behavior consistent with Safari, simplifies
the code, fixes a bug, and is more useful IMO. If you want to open
several tabs with variations on some search, for example, you can press
command-return after each, then keep typing to produce the next.

See discussion in the bug for more info.

BUG=513966
Review-Url: https://codereview.chromium.org/2904243002
Cr-Commit-Position: refs/heads/master@{#475542}
Committed: https://chromium.googlesource.com/chromium/src/+/1f35c3ffdebd60917647555ac6ec572098967035

[Justin Donnelly, 2017-05-26 17:53:22]

The CQ bit was checked by jdonnelly@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-26 17:54:00]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Justin Donnelly, 2017-05-26 17:54:05]

Description was changed from

==========
Don't reset the omnibox state on command-return.

This was originally added in https://codereview.chromium.org/260008/,
motivated it appears by a desire to match Safari's behavior. Safari no
longer does this (the address field state is preserved on command-return
as the new tab is openeed in the background).

This change makes Chrome's behavior consistent with Safari, simplifies
the code, fixes a bug, and is more useful IMO. If you want to open
several tabs with variations on some search, for example, you can press
command-return after each, then keep typing to produce the next.

See discussion in the bug for more info.

BUG=513966
==========

to

==========
Don't reset the omnibox state on command-return.

This was originally added in https://codereview.chromium.org/260008/,
motivated it appears by a desire to match Safari's behavior. Safari no
longer does this (the address field state is preserved on command-return
as the new tab is openeed in the background).

This change makes Chrome's behavior consistent with Safari, simplifies
the code, fixes a bug, and is more useful IMO. If you want to open
several tabs with variations on some search, for example, you can press
command-return after each, then keep typing to produce the next.

See discussion in the bug for more info.

BUG=513966
==========

[Justin Donnelly, 2017-05-26 17:54:05]

jdonnelly@chromium.org changed reviewers:
+ rohitrao@chromium.org

[commit-bot: I haz the power, 2017-05-26 18:15:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-26 18:16:00]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2017-05-30 14:41:14]

lgtm

https://codereview.chromium.org/2904243002/diff/1/chrome/browser/ui/cocoa/omn...
File chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm (left):

https://codereview.chromium.org/2904243002/diff/1/chrome/browser/ui/cocoa/omn...
chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm:829: if (GetTextLength() ==
0)
This was a fix you added earlier this month?  Now that we no longer call
RevertAll(), does that remove the need for this special-cased fix?

[Justin Donnelly, 2017-05-30 15:32:48]

https://codereview.chromium.org/2904243002/diff/1/chrome/browser/ui/cocoa/omn...
File chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm (left):

https://codereview.chromium.org/2904243002/diff/1/chrome/browser/ui/cocoa/omn...
chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm:829: if (GetTextLength() ==
0)
On 2017/05/30 14:41:14, rohitrao (ping after 24h) wrote:
> This was a fix you added earlier this month?  Now that we no longer call
> RevertAll(), does that remove the need for this special-cased fix?

Yes, that's correct. This was just to avoid calling RevertAll(). Now that we
don't call that, this isn't necessary. AcceptInput() is a no-op if there's no
input.

[Justin Donnelly, 2017-05-30 15:32:58]

The CQ bit was checked by jdonnelly@chromium.org

[commit-bot: I haz the power, 2017-05-30 15:33:10]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 15:55:33]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496158378625700, "parent_rev":
"aa76e566cab7436963a2d12312173cbd0339093d", "commit_rev":
"1f35c3ffdebd60917647555ac6ec572098967035"}

[commit-bot: I haz the power, 2017-05-30 15:55:41]

Description was changed from

==========
Don't reset the omnibox state on command-return.

This was originally added in https://codereview.chromium.org/260008/,
motivated it appears by a desire to match Safari's behavior. Safari no
longer does this (the address field state is preserved on command-return
as the new tab is openeed in the background).

This change makes Chrome's behavior consistent with Safari, simplifies
the code, fixes a bug, and is more useful IMO. If you want to open
several tabs with variations on some search, for example, you can press
command-return after each, then keep typing to produce the next.

See discussion in the bug for more info.

BUG=513966
==========

to

==========
Don't reset the omnibox state on command-return.

This was originally added in https://codereview.chromium.org/260008/,
motivated it appears by a desire to match Safari's behavior. Safari no
longer does this (the address field state is preserved on command-return
as the new tab is openeed in the background).

This change makes Chrome's behavior consistent with Safari, simplifies
the code, fixes a bug, and is more useful IMO. If you want to open
several tabs with variations on some search, for example, you can press
command-return after each, then keep typing to produce the next.

See discussion in the bug for more info.

BUG=513966

Review-Url: https://codereview.chromium.org/2904243002
Cr-Commit-Position: refs/heads/master@{#475542}
Committed:
https://chromium.googlesource.com/chromium/src/+/1f35c3ffdebd60917647555ac6ec...
==========

[commit-bot: I haz the power, 2017-05-30 15:55:42]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/1f35c3ffdebd60917647555ac6ec...

[Justin Donnelly, 2017-07-20 19:19:20]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2983193002/ by jdonnelly@chromium.org.

The reason for reverting is: This change caused a new issue:
https://crbug.com/743288. While we decide how to proceed, it would be better to
revert to the previous behavior..