Exclude readonly and disabled elements from autofill form.

Exclude readonly and disabled elements from autofill form.

R=isherman@chromium.org
BUG=231160
Committed: https://crrev.com/cfe1912bcc70118364c9156e1e12c6c89b1a06b9
Cr-Commit-Position: refs/heads/master@{#298436}

[ziran.sun, 2014-10-02 12:46:48]

Please review. Thanks!

[Mike West, 2014-10-02 12:49:57]

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

[Mike West, 2014-10-02 12:49:57]

https://codereview.chromium.org/621503003/diff/20001/components/autofill/cont...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/621503003/diff/20001/components/autofill/cont...
components/autofill/content/renderer/form_autofill_util.cc:730:
!element.isEnabled())
Why not fold this into IsAutofillable{Input?}Element?

[Ilya Sherman, 2014-10-02 21:16:26]

LGTM, thanks.

[ziran.sun, 2014-10-07 09:13:20]

The CQ bit was checked by ziran.sun@samsung.com

[commit-bot: I haz the power, 2014-10-07 09:14:28]

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

[commit-bot: I haz the power, 2014-10-07 10:20:30]

Committed patchset #3 (id:40001) as c4f574396a3000880b804904b125442b2214810b

[commit-bot: I haz the power, 2014-10-07 10:21:15]

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

[Evan Stade, 2014-10-13 18:31:03]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/652783002/ by estade@chromium.org.

The reason for reverting is: breaks requestAutocomplete.

[Ilya Sherman, 2014-10-13 21:08:17]

On 2014/10/13 18:31:03, Evan Stade wrote:
> A revert of this CL (patchset #3 id:40001) has been created in
> https://codereview.chromium.org/652783002/ by mailto:estade@chromium.org.
> 
> The reason for reverting is: breaks requestAutocomplete.

Why is requestAutocomplete filling readonly and disabled fields?

[Evan Stade, 2014-10-13 21:14:37]

On 2014/10/13 21:08:17, Ilya Sherman wrote:
> On 2014/10/13 18:31:03, Evan Stade wrote:
> > A revert of this CL (patchset #3 id:40001) has been created in
> > https://codereview.chromium.org/652783002/ by mailto:estade@chromium.org.
> > 
> > The reason for reverting is: breaks requestAutocomplete.
> 
> Why is requestAutocomplete filling readonly and disabled fields?

It needs to parse them, not fill them. I'm not sure if it does fill them right
now, and I don't have a strong opinion about whether it should (what is the use
case for putting a disabled field in a rAc form as opposed to just omitting it
or ignoring its value?).

See https://codereview.chromium.org/650293002/ for a use case for readonly.
Similarly, it should be possible to have a field in the form:

  <input value="Visa" autocomplete="cc-type" readonly>

Which would limit rAc to Visa cards. This doesn't work at the moment, but
arguably should.

[Ilya Sherman, 2014-10-13 21:16:17]

On 2014/10/13 21:14:37, Evan Stade wrote:
> On 2014/10/13 21:08:17, Ilya Sherman wrote:
> > On 2014/10/13 18:31:03, Evan Stade wrote:
> > > A revert of this CL (patchset #3 id:40001) has been created in
> > > https://codereview.chromium.org/652783002/ by mailto:estade@chromium.org.
> > > 
> > > The reason for reverting is: breaks requestAutocomplete.
> > 
> > Why is requestAutocomplete filling readonly and disabled fields?
> 
> It needs to parse them, not fill them. I'm not sure if it does fill them right
> now, and I don't have a strong opinion about whether it should (what is the
use
> case for putting a disabled field in a rAc form as opposed to just omitting it
> or ignoring its value?).
> 
> See https://codereview.chromium.org/650293002/ for a use case for readonly.
> Similarly, it should be possible to have a field in the form:
> 
>   <input value="Visa" autocomplete="cc-type" readonly>
> 
> Which would limit rAc to Visa cards. This doesn't work at the moment, but
> arguably should.

Yeah, that makes sense.  Thanks for adding test coverage :)