Mac: Don't replace "quotes" with “smartquotes” in the Omnibox

Mac: Don't replace "quotes" with “smartquotes” in the Omnibox

Actually, disable all substitutions be default. They're not likely to be
useful for the omnibox.

Since linking to the 10.10 SDK, the omnibox on Mac has started
smartquotifying input. It also replaces "--" with an em-dash. These are
unlikely to be desired for URL or search input, so this restores the old
behavior.

BUG=528014
TEST=Type a quote (") or two dashes (--) in the Omnibox on Mac and wait
a second. It shouldn't get replaced by a different character.

Committed: https://crrev.com/39a9074f2fc832be61d1332af1175cfae8bf9186
Cr-Commit-Position: refs/heads/master@{#348783}

[tapted, 2015-09-09 06:59:36]

tapted@chromium.org changed reviewers:
+ rsesek@chromium.org

[tapted, 2015-09-09 07:03:20]

Hi Robert, please take a look.

I poked around some of the other NSTextCheckingType options -- they don't seem
to be coming into play, nor do they appear to readily mess with a URL/search.

(note I'm OOO for a few days, so if it looks good, please CQ).

Thanks!

[Scott Hess - ex-Googler, 2015-09-09 15:55:04]

shess@chromium.org changed reviewers:
+ shess@chromium.org

[Scott Hess - ex-Googler, 2015-09-09 15:55:04]

https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
(right):

https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:67:
checkingTypes &= ~NSTextCheckingTypeDash;
I support going for broke and disabling all intrusive automagic changes.  I
don't know if that means setting it to 0, or &= NSTextCheckingAllTypes.

[Robert Sesek, 2015-09-09 22:38:25]

https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
(right):

https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:67:
checkingTypes &= ~NSTextCheckingTypeDash;
On 2015/09/09 15:55:04, Scott Hess wrote:
> I support going for broke and disabling all intrusive automagic changes.  I
> don't know if that means setting it to 0, or &= NSTextCheckingAllTypes.

I'm OK with this, too, though. I don't feel strongly either way.

[groby-ooo-7-16, 2015-09-12 00:53:45]

groby@chromium.org changed reviewers:
+ groby@chromium.org

[groby-ooo-7-16, 2015-09-12 00:53:46]

https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
(right):

https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:67:
checkingTypes &= ~NSTextCheckingTypeDash;
On 2015/09/09 22:38:25, Robert Sesek wrote:
> On 2015/09/09 15:55:04, Scott Hess wrote:
> > I support going for broke and disabling all intrusive automagic changes.  I
> > don't know if that means setting it to 0, or &= NSTextCheckingAllTypes.
> 
> I'm OK with this, too, though. I don't feel strongly either way.

FWIW, Safari lets you pick which substitutions you want. But I don't think any
should be on by default

[Scott Hess - ex-Googler, 2015-09-12 01:27:25]

On 2015/09/12 00:53:46, groby wrote:
>
https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
> File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
> (right):
> 
>
https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
> chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:67:
> checkingTypes &= ~NSTextCheckingTypeDash;
> On 2015/09/09 22:38:25, Robert Sesek wrote:
> > On 2015/09/09 15:55:04, Scott Hess wrote:
> > > I support going for broke and disabling all intrusive automagic changes. 
I
> > > don't know if that means setting it to 0, or &= NSTextCheckingAllTypes.
> > 
> > I'm OK with this, too, though. I don't feel strongly either way.
> 
> FWIW, Safari lets you pick which substitutions you want. But I don't think any
> should be on by default

I think for any of the "replaces" set, which is what we're swapping out here, we
definitely don't want it.

Some of the "checks" cases, I can see.  Having it auto-correct spelling would be
bad, but having it flag speling and let you right-click to fix seems reasonable.

I'm unclear what matching a date or phone number or location means.  Does it
give a right-click context menu for it?  But if it's just the let you search
your address book or locate on a map, that seems silly - either the omnibox is
already going to provide similar functionality, or you can go type somewhere
else.

Maybe a better option than either zeroing this out or the current blacklist
would be an explicit whitelist.  Then it will get a few features we allow,
without acquiring new features which break things on a new OS release.

[groby-ooo-7-16, 2015-09-12 01:37:31]

On 2015/09/12 01:27:25, Scott Hess wrote:
> On 2015/09/12 00:53:46, groby wrote:
> >
>
https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
> > File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
> > (right):
> > 
> >
>
https://codereview.chromium.org/1320093005/diff/1/chrome/browser/ui/cocoa/loc...
> > chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:67:
> > checkingTypes &= ~NSTextCheckingTypeDash;
> > On 2015/09/09 22:38:25, Robert Sesek wrote:
> > > On 2015/09/09 15:55:04, Scott Hess wrote:
> > > > I support going for broke and disabling all intrusive automagic changes.

> I
> > > > don't know if that means setting it to 0, or &= NSTextCheckingAllTypes.
> > > 
> > > I'm OK with this, too, though. I don't feel strongly either way.
> > 
> > FWIW, Safari lets you pick which substitutions you want. But I don't think
any
> > should be on by default
> 
> I think for any of the "replaces" set, which is what we're swapping out here,
we
> definitely don't want it.
> 
> Some of the "checks" cases, I can see.  Having it auto-correct spelling would
be
> bad, but having it flag speling and let you right-click to fix seems
reasonable.
> 
> I'm unclear what matching a date or phone number or location means.  Does it
> give a right-click context menu for it?  But if it's just the let you search
> your address book or locate on a map, that seems silly - either the omnibox is
> already going to provide similar functionality, or you can go type somewhere
> else.
> 
> Maybe a better option than either zeroing this out or the current blacklist
> would be an explicit whitelist.  Then it will get a few features we allow,
> without acquiring new features which break things on a new OS release.

The main point I think is that most OSX text fields have a "Substitutions" sub
menu, where it's up to the user to pick what they want. We don't have that here.

Unless we want to expose that control, I think we should start with an empty
whitelist and consider very carefully if individual cases make sense in the
Omnibox. (I don't think spellcheck does. There's a server or three sitting in a
datacenter doing that anyways :)

[tapted, 2015-09-14 07:31:22]

Well I set it to 0, had a tinker, and nothing exploded :). PTAL.

I agree that there don't seem to be any substitution categories that would be
useful for the omnibox.

Note this will likely need a merge - one could argue that patchset 1 is the more
conservative option, but there could just as easily be something obscure we are
missing too, so just setting to 0 feels right to me.

[groby-ooo-7-16, 2015-09-14 14:35:31]

LGTM from my POV. Please wait for shess, he seemed to have some interest in
non-0 types.

[Scott Hess - ex-Googler, 2015-09-14 15:19:13]

On 2015/09/14 14:35:31, groby wrote:
> LGTM from my POV. Please wait for shess, he seemed to have some interest in
> non-0 types.

LGTM, I'm just peanut-gallery-sniping.  More than happy to wait for someone to
log a bug about how some obscure spell-check feature doesn't work in this field
...

[tapted, 2015-09-15 00:00:40]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2015-09-15 00:01:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1320093005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1320093005/20001

[commit-bot: I haz the power, 2015-09-15 00:08:33]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-15 00:09:05]

Patchset 2 (id:??) landed as
https://crrev.com/39a9074f2fc832be61d1332af1175cfae8bf9186
Cr-Commit-Position: refs/heads/master@{#348783}

[commit-bot: I haz the power, 2015-09-23 12:39:54]

Patchset 2 (id:??) landed as
https://crrev.com/39a9074f2fc832be61d1332af1175cfae8bf9186
Cr-Commit-Position: refs/heads/master@{#348783}