Password length enforcement

Password length enforcement


BUG=120608
TBR=sky@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149785

[Harry McCleave, 2012-08-01 18:53:56]

could you take a look at this and see if it's a reasonable solution to the issue
identified.

[James Cook, 2012-08-02 20:50:29]

On 2012/08/01 18:53:56, Harry McCleave wrote:
> could you take a look at this and see if it's a reasonable solution to the
issue
> identified.

I don't think this is right.  The enabled status of passphrase_textfield_ is set
based on the result of PassphraseActive(), but then you're modifying the result
of PassphraseActive() based on its enabled status.  That doesn't seem right.

I wonder if you should modify WifiConfigView::CanLogin() to check for the sort
of network that needs the length check, then check the text length.  If you
don't know the network type you can't enforce this limit, but it sounds in the
bug like the dialog already knows this is WPA or WEP?

Sorry I can't be more specific, but it's been over a year since I've been in
this code.  If you need more help stevenjb@ might have some advice.

[stevenjb, 2012-08-02 21:48:13]

I this actually this looks fine. We're just enforcing a minimum passphrase for
the UI here as a convenient indicator to the user. If the field is not enabled,
it does not make sense to do the minimum length check here.
LGTM

[stevenjb, 2012-08-02 21:49:01]

To clarify, the second patch looks fine. Yeah, the first patch would have been
problematic.

[commit-bot: I haz the power, 2012-08-02 21:53:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/harrym@chromium.org/10824131/4001

[commit-bot: I haz the power, 2012-08-02 21:53:37]

Presubmit check for 10824131-4001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

[Nico, 2012-08-02 22:13:15]

james / steven: Harry asked me to OWNERS-review this, and I in turn suggested
adding an OWNERS file for chrome/browser/chromeos (or event …/options). Who
would be good owners for this file? Can one of you two create it?

[stevenjb, 2012-08-02 23:58:06]

OWNERS file review is here, awaiting zel to push it:
https://chromiumcodereview.appspot.com/10830103/

In the meanwhile you can TBR= this.

[commit-bot: I haz the power, 2012-08-03 00:04:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/harrym@chromium.org/10824131/4001

[commit-bot: I haz the power, 2012-08-03 02:32:29]

Change committed as 149785