Add an "input field type" entry to WebContextMenuData.

Add an "input field type" entry to WebContextMenuData.

BUG=484891
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195534

[msramek, 2015-05-19 12:40:04]

msramek@chromium.org changed reviewers:
+ mkwst@chromium.org

[msramek, 2015-05-19 12:40:05]

Hi Mike,

as per your suggestion, I moved the input field analysis to blink. PTAL.

Thanks,
Martin

[Mike West, 2015-05-19 12:58:35]

It would be excellent to start a test suite for this file. Sorry that no one's
done that yet. :/ If you feel inspired, please poke at it (you can find lots of
`*Test.cpp` examples in `Source/web` (well... "lots")).

Otherwise LGTM % the autocomplete nit.

https://codereview.chromium.org/1137733004/diff/1/Source/web/ContextMenuClien...
File Source/web/ContextMenuClientImpl.cpp (right):

https://codereview.chromium.org/1137733004/diff/1/Source/web/ContextMenuClien...
Source/web/ContextMenuClientImpl.cpp:401: if (element->type() ==
InputTypeNames::password)
Should we also take the `autocomplete` attribute into consideration here? If so,
we could pick up "username", for instance.

[msramek, 2015-05-19 15:42:45]

Thanks, Mike!

I'll add some tests in a follow-up CL. I'm landing this for now, so that trybots
will be able to compile the other CL.

https://codereview.chromium.org/1137733004/diff/1/Source/web/ContextMenuClien...
File Source/web/ContextMenuClientImpl.cpp (right):

https://codereview.chromium.org/1137733004/diff/1/Source/web/ContextMenuClien...
Source/web/ContextMenuClientImpl.cpp:401: if (element->type() ==
InputTypeNames::password)
On 2015/05/19 12:58:35, Mike West (holiday in DE) wrote:
> Should we also take the `autocomplete` attribute into consideration here? If
so,
> we could pick up "username", for instance.

I thought about it a bit, and I would say that not just yet. I'll keep it in
mind for future iterations :)

We could define InputFieldTypeUsername and show some special context menu item
on fields with autocomplete="username". However, then we'll also want the same
behavior for forms that do not use autocomplete. We could say that if no field
in the form has autocomplete="username", then all of them will be treated as
InputFieldTypeUsername.

This might be useful, but it seems like inconsistently rewriting the password
manager heuristics in blink, and I would avoid it until we have clear idea what
exactly we want to offer for the username field. The current design expects no
action on the username field; the username value will be found using the usual
heuristics.

About autocomplete="current-password" and "new-password", I had a quick chat
with Václav and we arrived to the conclusion that we don't want to discriminate;
the user using the force-save feature knows what they're doing, and they should
decide which password field is correct themselves.

[msramek, 2015-05-19 15:42:51]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2015-05-19 15:43:14]

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

[commit-bot: I haz the power, 2015-05-19 17:09:32]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195534