AutoFill fills only clicked field when form was filled prior.

AutoFill fills only clicked field when form was filled prior.

These changes make it so that individual fields are filled with the value selected from
the input element popup menu.  AutoFill profile data is not queried in this case.

BUG=44618, 47917
TEST=RenderViewTest.SendForms, RenderViewTest.FillFormElement

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

[James Hawkins, 2010-06-29 21:04:46]

http://codereview.chromium.org/2808036/diff/1/2
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/2808036/diff/1/2#newcode2114
chrome/renderer/render_view.cc:2114: } else if (label.isEmpty()) {
This should be based on the auto-filled status of the form, not whether there's
a label or not. The assumption is not very stable.

[dhollowa, 2010-06-29 22:19:51]

http://codereview.chromium.org/2808036/diff/1/2
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/2808036/diff/1/2#newcode2114
chrome/renderer/render_view.cc:2114: } else if (label.isEmpty()) {
On 2010/06/29 21:04:46, James Hawkins wrote:
> This should be based on the auto-filled status of the form, not whether
there's
> a label or not. The assumption is not very stable.

Done.  Better, yes.

[James Hawkins, 2010-06-29 22:23:19]

http://codereview.chromium.org/2808036/diff/6001/7001
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/2808036/diff/6001/7001#newcode2116
chrome/renderer/render_view.cc:2116: WebInputElement element =
node.toConst<WebInputElement>();
You use toConst, but aren't using a const element. You want node.to<...>();

http://codereview.chromium.org/2808036/diff/6001/7001#newcode2119
chrome/renderer/render_view.cc:2119: // |label| is set so fill the query values
for the whole form.
I think this comment is now stale.

http://codereview.chromium.org/2808036/diff/6001/7003
File chrome/renderer/render_view_unittest.cc (right):

http://codereview.chromium.org/2808036/diff/6001/7003#newcode1034
chrome/renderer/render_view_unittest.cc:1034: "  <input type=\"hidden\"
id=\"lastname\"/>"
Is a 'hidden' field necessary for this test?

http://codereview.chromium.org/2808036/diff/6001/7003#newcode1078
chrome/renderer/render_view_unittest.cc:1078: // Accept a suggestion with empty
label.  This triggers the direct filling of
This test is based on the 'empty label' implementation, should be changed to
auto-fill the form first, clear a field, then attempt to fill one field.

[dhollowa, 2010-06-29 22:43:54]

http://codereview.chromium.org/2808036/diff/6001/7001
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/2808036/diff/6001/7001#newcode2116
chrome/renderer/render_view.cc:2116: WebInputElement element =
node.toConst<WebInputElement>();
On 2010/06/29 22:23:19, James Hawkins wrote:
> You use toConst, but aren't using a const element. You want node.to<...>();

node.to<...>() doesn't cast away const'ness.

http://codereview.chromium.org/2808036/diff/6001/7001#newcode2119
chrome/renderer/render_view.cc:2119: // |label| is set so fill the query values
for the whole form.
On 2010/06/29 22:23:19, James Hawkins wrote:
> I think this comment is now stale.

Done.

http://codereview.chromium.org/2808036/diff/6001/7003
File chrome/renderer/render_view_unittest.cc (right):

http://codereview.chromium.org/2808036/diff/6001/7003#newcode1034
chrome/renderer/render_view_unittest.cc:1034: "  <input type=\"hidden\"
id=\"lastname\"/>"
On 2010/06/29 22:23:19, James Hawkins wrote:
> Is a 'hidden' field necessary for this test?

Done.  Not needed at all actually.  Removed.

http://codereview.chromium.org/2808036/diff/6001/7003#newcode1078
chrome/renderer/render_view_unittest.cc:1078: // Accept a suggestion with empty
label.  This triggers the direct filling of
On 2010/06/29 22:23:19, James Hawkins wrote:
> This test is based on the 'empty label' implementation, should be changed to
> auto-fill the form first, clear a field, then attempt to fill one field.

It is not actually, just a stale comment.  The call above to
|middlename.setAutofilled(true)| has the same effect as you mention.

[James Hawkins, 2010-06-29 22:46:31]

LGTM with added comment.

http://codereview.chromium.org/2808036/diff/6001/7001
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/2808036/diff/6001/7001#newcode2116
chrome/renderer/render_view.cc:2116: WebInputElement element =
node.toConst<WebInputElement>();
On 2010/06/29 22:43:54, dhollowa wrote:
> On 2010/06/29 22:23:19, James Hawkins wrote:
> > You use toConst, but aren't using a const element. You want node.to<...>();
> 
> node.to<...>() doesn't cast away const'ness.

Gotcha. Please add a comment for this.