Issue 5280001: Autofill i18n: Develop a system for validating Autofill heuristics from HTML input

Issue 5280001: Autofill i18n: Develop a system for validating Autofill heuristics from HTML input (Closed)

Created:
10 years, 1 month ago by dhollowa

Modified:
9 years, 7 months ago

Reviewers:
vivianz, Ilya Sherman

CC:
chromium-reviews, Paweł Hajdan Jr., ben+cc_chromium.org, James Hawkins, dhollowa

Base URL:
svn://svn.chromium.org/chrome/trunk/src/chrome

Visibility:
Public.

Description

Autofill i18n: Develop a system for validating Autofill heuristics from HTML input Adds form_structure_browsertest.cc, an InProcessBrowserTest that serializes the parsed FormStructure objects and compares the results in string form. This is the first step towards a system that reads HTML forms from disk and compares against known results. BUG=60022 TEST=FormStructureBrowserTest.BasicFormStructure

Patch Set 1 #

Total comments: 6

Patch Set 2 : Addressing review comments. #

Patch Set 3 : Typo. #

Created: 10 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+218 lines, -0 lines)			Patch
M	chrome/browser/autofill/autofill_manager.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
A	chrome/browser/autofill/form_structure_browsertest.cc	View	1 2	1 chunk	+216 lines, -0 lines	0 comments	Download
M	chrome/chrome_tests.gypi	View	1	1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Ilya Sherman

LGTM, thanks http://codereview.chromium.org/5280001/diff/1/chrome/browser/autofill/form_structure_browsertest.cc File chrome/browser/autofill/form_structure_browsertest.cc (right): http://codereview.chromium.org/5280001/diff/1/chrome/browser/autofill/form_structure_browsertest.cc#newcode61 chrome/browser/autofill/form_structure_browsertest.cc:61: if (!*field_iter) nit: Would be nice to ...

10 years, 1 month ago (2010-11-23 00:45:24 UTC) #3

dhollowa

10 years, 1 month ago (2010-11-23 01:10:09 UTC) #4

http://codereview.chromium.org/5280001/diff/1/chrome/browser/autofill/form_st...
File chrome/browser/autofill/form_structure_browsertest.cc (right):

http://codereview.chromium.org/5280001/diff/1/chrome/browser/autofill/form_st...
chrome/browser/autofill/form_structure_browsertest.cc:61: if (!*field_iter)
On 2010/11/23 00:45:24, Ilya Sherman wrote:
> nit: Would be nice to add a comment explaining why we need this if-stmt --
it's
> because the AutoFillField vector is NULL-terminated, right?

Done.

http://codereview.chromium.org/5280001/diff/1/chrome/browser/autofill/form_st...
chrome/browser/autofill/form_structure_browsertest.cc:195: "<form
action=\"http://www.google.com/\" method=\"POST\">"
On 2010/11/23 00:45:24, Ilya Sherman wrote:
> nit: 80-col

Done.

http://codereview.chromium.org/5280001/diff/1/chrome/browser/autofill/form_st...
chrome/browser/autofill/form_structure_browsertest.cc:214: EXPECT_EQ(1UL,
forms.size());
On 2010/11/23 00:45:24, Ilya Sherman wrote:
> nit: this seems unnecessary given the string comparison above

Done.

Expand Messages | Collapse Messages