Autofill OSX: Disable inputs and Verify button after permanent error.

Autofill OSX: Disable inputs and Verify button after permanent error.

Disable expiration date popups, CVC text input field, and storage
checkbox once a permanent error message is shown.

BUG=448572
Committed: https://crrev.com/d15c28fdcdce11295eb1a8ea961a99cc2d6622bf
Cr-Commit-Position: refs/heads/master@{#322490}

[bondd, 2015-03-26 19:50:06]

bondd@chromium.org changed reviewers:
+ estade@chromium.org, groby@chromium.org

[bondd, 2015-03-26 19:50:06]

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

[bondd, 2015-03-26 19:50:07]

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
File chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm (right):

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:202: -
(void)setInputsEnabled:(BOOL)enabled {
Only ever called with enabled == NO. I could rename it to -disableInputs if you
prefer.

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:252:
![[permanentErrorLabel_ stringValue] length] &&
Alternatively, this line could be "[cvcInput_ isEnabled] &&"

[groby-ooo-7-16, 2015-03-26 20:17:45]

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
File chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm (right):

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:202: -
(void)setInputsEnabled:(BOOL)enabled {
On 2015/03/26 19:50:06, bondd wrote:
> Only ever called with enabled == NO. I could rename it to -disableInputs if
you
> prefer.

No, I'd leave it, for API symmetry. If you leave this as a separate call.

but - why not:

for (NSView view in [[self view] subviews])
  [view setEnabled:enabled];

which could be neatly inlined.

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:252:
![[permanentErrorLabel_ stringValue] length] &&
On 2015/03/26 19:50:06, bondd wrote:
> Alternatively, this line could be "[cvcInput_ isEnabled] &&"

I like the current one better, because it's more general. The cvcInput will
break if there are ever additional cases that can set an error

[bondd, 2015-03-26 20:33:12]

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
File chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm (right):

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:202: -
(void)setInputsEnabled:(BOOL)enabled {
On 2015/03/26 20:17:45, groby wrote:
> On 2015/03/26 19:50:06, bondd wrote:
> > Only ever called with enabled == NO. I could rename it to -disableInputs if
> you
> > prefer.
> 
> No, I'd leave it, for API symmetry. If you leave this as a separate call.
> 
> but - why not:
> 
> for (NSView view in [[self view] subviews])
>   [view setEnabled:enabled];
> 
> which could be neatly inlined.

We want to leave the Cancel button enabled.

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:252:
![[permanentErrorLabel_ stringValue] length] &&
On 2015/03/26 20:17:45, groby wrote:
> On 2015/03/26 19:50:06, bondd wrote:
> > Alternatively, this line could be "[cvcInput_ isEnabled] &&"
> 
> I like the current one better, because it's more general. The cvcInput will
> break if there are ever additional cases that can set an error

Acknowledged.

[groby-ooo-7-16, 2015-03-26 20:40:44]

lgtm

[bondd, 2015-03-26 20:46:52]

Hit reply too soon. Replying to my reply. See below.

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
File chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm (right):

https://codereview.chromium.org/1038043003/diff/1/chrome/browser/ui/cocoa/aut...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:202: -
(void)setInputsEnabled:(BOOL)enabled {
On 2015/03/26 20:33:12, bondd wrote:
> On 2015/03/26 20:17:45, groby wrote:
> > On 2015/03/26 19:50:06, bondd wrote:
> > > Only ever called with enabled == NO. I could rename it to -disableInputs
if
> > you
> > > prefer.
> > 
> > No, I'd leave it, for API symmetry. If you leave this as a separate call.
> > 
> > but - why not:
> > 
> > for (NSView view in [[self view] subviews])
> >   [view setEnabled:enabled];
> > 
> > which could be neatly inlined.
> 
> We want to leave the Cancel button enabled.

Also, unless I'm missing something this will give you compiler
warning-treated-as-error
"error: 'NSView' may not respond to 'setEnabled:'"
and will not touch any of these UI elements because they are all contained in
other NSViews (inputRowView_ and storageView_). But like I said, maybe I am
missing something here...it's certainly happened before. :)

[groby-ooo-7-16, 2015-03-26 20:59:31]

No, you're right and I spaced. You'd be able to cast it to an NSControl, but at
that point, it's ugly :)

[bondd, 2015-03-26 22:24:09]

The CQ bit was checked by bondd@chromium.org

[commit-bot: I haz the power, 2015-03-26 22:24:23]

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

[commit-bot: I haz the power, 2015-03-26 22:57:32]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-03-26 22:58:33]

Patchset 1 (id:??) landed as
https://crrev.com/d15c28fdcdce11295eb1a8ea961a99cc2d6622bf
Cr-Commit-Position: refs/heads/master@{#322490}