Improve Autofill heuristics when detecting labels from previous elements.

Improve Autofill heuristics when detecting labels from previous elements.

Support HTML like """Name <span class="required">*</span> <input type="text" name="name">"""

Of course, pull at a thread and...

Other changes also included to avoid regressions:

 * When parsing address fields for heuristics, we try to skip over unlabeled fields in the middle of an address.  Updated the code not to also skip over unlabeled fields at the *end* of an address, as these might be part of a different section entirely.

 * Tighten the credit card number regex to require the word "card".

 * Add "csc" to the credit card security code regex.

 * When inferring labels based on <div> structure, be willing to scan up the tree past the closest parent that is a <div>.

 * Also when inferring labels based on <div> structure, we previously would only stop early if we were about to escape from a <table> element.  Also stop early if we are about to escape from a <fieldset> element.  If we see either of these elements, we expect the field label to be contained with the element.

 * Allow <label> elements to misuse the 'for' attribute to specify the element's name rather than its id.

BUG=87517
TEST=browser_tests --gtest_filter=FormStructureBrowserTest.DataDrivenHeuristics*

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

[dhollowa, 2011-08-01 22:16:53]

LGTM.  Pending comment requests and small readability request.  These seem like
very positive changes from a heuristics point of view.  Thanks.

http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
File chrome/renderer/autofill/form_manager.cc (right):

http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
chrome/renderer/autofill/form_manager.cc:106: // has leading whitespace.
Some examples in this comment would help the casual reader.  The comment as it
stands is on par with directly reading the code.  ;-)

http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
chrome/renderer/autofill/form_manager.cc:221: } else if
(previous.isElementNode()) {
Instead of "else if", it would be more readable IMO if you pulled the |break|
into subsequent "if" blocks.  More like the original code.  And then pull the
"All other elements are..." comment up above the second "if".

[Ilya Sherman, 2011-08-01 23:59:35]

http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
File chrome/renderer/autofill/form_manager.cc (right):

http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
chrome/renderer/autofill/form_manager.cc:106: // has leading whitespace.
On 2011/08/01 22:16:54, dhollowa wrote:
> Some examples in this comment would help the casual reader.  The comment as it
> stands is on par with directly reading the code.  ;-)

Done.

http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
chrome/renderer/autofill/form_manager.cc:221: } else if
(previous.isElementNode()) {
On 2011/08/01 22:16:54, dhollowa wrote:
> Instead of "else if", it would be more readable IMO if you pulled the |break|
> into subsequent "if" blocks.  More like the original code.  And then pull the
> "All other elements are..." comment up above the second "if".
>  

Done.

[dhollowa, 2011-08-02 00:24:19]

SLGTM.  Thanks.

On 2011/08/01 23:59:35, Ilya Sherman wrote:
>
http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
> File chrome/renderer/autofill/form_manager.cc (right):
> 
>
http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
> chrome/renderer/autofill/form_manager.cc:106: // has leading whitespace.
> On 2011/08/01 22:16:54, dhollowa wrote:
> > Some examples in this comment would help the casual reader.  The comment as
it
> > stands is on par with directly reading the code.  ;-)
> 
> Done.
> 
>
http://codereview.chromium.org/7531023/diff/3001/chrome/renderer/autofill/for...
> chrome/renderer/autofill/form_manager.cc:221: } else if
> (previous.isElementNode()) {
> On 2011/08/01 22:16:54, dhollowa wrote:
> > Instead of "else if", it would be more readable IMO if you pulled the
|break|
> > into subsequent "if" blocks.  More like the original code.  And then pull
the
> > "All other elements are..." comment up above the second "if".
> >  
> 
> Done.