Change the login name auto completion logic to case sensitive....

Change the login name autocompletion logic to case sensitive.

Contributed by Takano.Naoki@gmail.com (honten)

BUG=55874
TEST=Assume the user's login name is 'Chris' and once it's saved, the autocompletion is not triggered if the start charactor is 'c'. The autocompletion is triggered iff the start charactor is 'C'.

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

[honten.org, 2010-11-20 06:53:20]

I fixed it and wrote the test. Could you review it?

If the test is not enough, please let me know.

[honten.org, 2010-11-23 04:09:01]

Could you review?

Or are you off because of holiday?

[Ilya Sherman, 2010-11-23 07:16:52]

On 2010/11/23 04:09:01, honten wrote:
> Could you review?
> 
> Or are you off because of holiday?

Sorry, had a busy day today, so just now getting to this.  LGTM with the changes
below.

I'm also adding Jay to make sure he's ok with changing this logic to be
case-sensitive.

http://codereview.chromium.org/5197005/diff/1/chrome/renderer/password_autoco...
File chrome/renderer/password_autocomplete_manager_unittest.cc (right):

http://codereview.chromium.org/5197005/diff/1/chrome/renderer/password_autoco...
chrome/renderer/password_autocomplete_manager_unittest.cc:41: const char* const
kChrisUsername = "Chris";
nit: In the spirit of cryptographic silliness, please use of the names listed
here: http://en.wikipedia.org/wiki/Alice_and_Bob#List_of_characters =)

http://codereview.chromium.org/5197005/diff/1/chrome/renderer/password_autoco...
chrome/renderer/password_autocomplete_manager_unittest.cc:297: // charactoer is
'C'.
nit: This comment would match the surrounding comments better if it were
something like

"""
// Then, the user again removes all the text and types an uppercase 'C'.
SimulateUsernameChange("C", true);
// The username and password fields should match the 'Chris' entry.
"""

"""
// Finally, the user removes all the text and types a lowercase 'c'.  We only
want case-sensitive autocompletion, so the username and the selected range
should be empty."
SimulateUsernameChange("c", true);
"""


Also, please move this code to be below the 'bob' case.

[honten.org, 2010-11-23 15:42:27]

llya,

Thank you for your comment.
I'm glad to hear your LGTM.

Tonight, I'll upload it again.

[Jay Civelli, 2010-11-23 17:56:25]

OK

On Tue, Nov 23, 2010 at 7:42 AM,  <Takano.Naoki@gmail.com> wrote:
> llya,
>
> Thank you for your comment.
> I'm glad to hear your LGTM.
>
> Tonight, I'll upload it again.
>
> http://codereview.chromium.org/5197005/
>

[honten.org, 2010-11-24 02:53:02]

Could you review again?

And if it's Ok, please commit them.

Thanks,

On 2010/11/23 17:56:25, Jay Civelli wrote:
> OK
> 
> On Tue, Nov 23, 2010 at 7:42 AM,  <mailto:Takano.Naoki@gmail.com> wrote:
> > llya,
> >
> > Thank you for your comment.
> > I'm glad to hear your LGTM.
> >
> > Tonight, I'll upload it again.
> >
> > http://codereview.chromium.org/5197005/
> >

[Ilya Sherman, 2010-11-24 07:40:11]

LGTM.  Running past the try bots just in case; will commit if all is green =)

[Ilya Sherman, 2010-11-24 09:05:39]

Committed as crrev.com/67240

http://codereview.chromium.org/5197005/diff/8001/chrome/renderer/password_aut...
File chrome/renderer/password_autocomplete_manager_unittest.cc (left):

http://codereview.chromium.org/5197005/diff/8001/chrome/renderer/password_aut...
chrome/renderer/password_autocomplete_manager_unittest.cc:313: }  // namespace
nit: We actually want the "  // namespace" -- it indicates the end of an
anonymous namespace.  Rather than make you go through another cycle on this cl,
I've gone ahead and readded this in http://codereview.chromium.org/5374001/

[takano.naoki_gmail.com, 2010-11-24 17:31:55]

Thanks!!

If you have any other bugs which look like I can try, please let me know.

On Wed, Nov 24, 2010 at 1:05 AM,  <isherman@chromium.org> wrote:
> Committed as crrev.com/67240
>
>
>
http://codereview.chromium.org/5197005/diff/8001/chrome/renderer/password_aut...
> File chrome/renderer/password_autocomplete_manager_unittest.cc (left):
>
>
http://codereview.chromium.org/5197005/diff/8001/chrome/renderer/password_aut...
> chrome/renderer/password_autocomplete_manager_unittest.cc:313: }  //
> namespace
> nit: We actually want the "  // namespace" -- it indicates the end of an
> anonymous namespace.  Rather than make you go through another cycle on
> this cl, I've gone ahead and readded this in
> http://codereview.chromium.org/5374001/
>
> http://codereview.chromium.org/5197005/
>