Jean-François Le (Gerrit)

unread,

Jan 20, 2026, 4:02:04 PM (21 hours ago) Jan 20

to Vincent Boisselle, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Vincent Boisselle

Jean-François Le added 1 comment

File components/autofill/ios/browser/autofill_agent_unittests.mm

Line 245, Patchset 1 (Latest): EXPECT_EQ(u"__gCrWeb.callFunctionInGcrWeb('autofill', 'fillForm', "

Jean-François Le . unresolved

All fields are sent and we also added the isAutofilled data.

Open in Gerrit

Related details

Attention is currently required from:

Vincent Boisselle

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Vincent Boisselle (Gerrit)

unread,

Jan 20, 2026, 4:17:20 PM (21 hours ago) Jan 20

to Jean-François Le, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Jean-François Le

Vincent Boisselle added 5 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Vincent Boisselle . resolved

approach lgtm, a few comments

Commit Message

Line 8, Patchset 1 (Latest):

Vincent Boisselle . unresolved

add a bit more details of this involves

my understanding is that we now allow filling empty filldata and unmark autofilled in the renderer

File components/autofill/ios/browser/autofill_agent_unittests.mm

Line 245, Patchset 1 (Latest): EXPECT_EQ(u"__gCrWeb.callFunctionInGcrWeb('autofill', 'fillForm', "

Jean-François Le . unresolved

All fields are sent and we also added the isAutofilled data.

Vincent Boisselle

ok so we only added isAutofilled to the fill call args basically ?

File ios/chrome/browser/autofill/model/autofill_java_script_feature_unittest.mm

Line 579, Patchset 1 (Latest): secondFieldData.Set("isAutofilled", YES);

Vincent Boisselle . unresolved

do we have a test case for validating the behavior when isAutofilled is NO ?

Line 603, Patchset 1 (Latest):

                @"document.getElementById('email')."
                @"hasAttribute('chrome-autofilled');",

Vincent Boisselle . unresolved

is this attr set async after FillForm() is done (when `block_was_called` == true) ?

otherwise you dont need to wait

Open in Gerrit

Related details

Attention is currently required from:

Jean-François Le

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Jean-François Le (Gerrit)

unread,

Jan 20, 2026, 4:33:17 PM (20 hours ago) Jan 20

to Vincent Boisselle, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Vincent Boisselle

Jean-François Le added 3 comments

Commit Message

Line 8, Patchset 1:

Vincent Boisselle . resolved

add a bit more details of this involves
my understanding is that we now allow filling empty filldata and unmark autofilled in the renderer

Jean-François Le

Done

File components/autofill/ios/browser/autofill_agent_unittests.mm

Line 245, Patchset 1: EXPECT_EQ(u"__gCrWeb.callFunctionInGcrWeb('autofill', 'fillForm', "

Jean-François Le . unresolved

All fields are sent and we also added the isAutofilled data.

Vincent Boisselle

ok so we only added isAutofilled to the fill call args basically ?

Jean-François Le

Yes and since we are not skipping empty fields and `autofilled == false` anymore, field 4 and 5 are added to the expected result.

File ios/chrome/browser/autofill/model/autofill_java_script_feature_unittest.mm

Line 603, Patchset 1: @"document.getElementById('email')."


                @"hasAttribute('chrome-autofilled');",

Vincent Boisselle . unresolved

is this attr set async after FillForm() is done (when `block_was_called` == true) ?
otherwise you dont need to wait

Jean-François Le

Without the wait the test was failing since the attribute is added in a window.setTimeOut.

Open in Gerrit

Related details

Attention is currently required from:

Vincent Boisselle

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Vincent Boisselle (Gerrit)

unread,

Jan 20, 2026, 4:50:32 PM (20 hours ago) Jan 20

to Jean-François Le, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Jean-François Le

Vincent Boisselle added 1 comment

File ios/chrome/browser/autofill/model/autofill_java_script_feature_unittest.mm

Line 603, Patchset 1:

                @"document.getElementById('email')."
                @"hasAttribute('chrome-autofilled');",

Vincent Boisselle . resolved

is this attr set async after FillForm() is done (when `block_was_called` == true) ?
otherwise you dont need to wait

Jean-François Le

Without the wait the test was failing since the attribute is added in a window.setTimeOut.

Vincent Boisselle

ah! I forgot about that

fillign results are returned but there is still that timeout before it actually happens

Open in Gerrit

Related details

Attention is currently required from:

Jean-François Le

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Jean-François Le (Gerrit)

unread,

11:05 AM (2 hours ago) 11:05 AM

to Vincent Boisselle, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Vincent Boisselle

Jean-François Le added 2 comments

File components/autofill/ios/browser/autofill_agent_unittests.mm

Line 245, Patchset 1: EXPECT_EQ(u"__gCrWeb.callFunctionInGcrWeb('autofill', 'fillForm', "

Jean-François Le . resolved

All fields are sent and we also added the isAutofilled data.

Vincent Boisselle

ok so we only added isAutofilled to the fill call args basically ?

Jean-François Le

Yes and since we are not skipping empty fields and `autofilled == false` anymore, field 4 and 5 are added to the expected result.

Jean-François Le

Done

File ios/chrome/browser/autofill/model/autofill_java_script_feature_unittest.mm

Line 579, Patchset 1: secondFieldData.Set("isAutofilled", YES);

Vincent Boisselle . resolved

do we have a test case for validating the behavior when isAutofilled is NO ?

Jean-François Le

Done

Open in Gerrit

Related details

Attention is currently required from:

Vincent Boisselle

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Vincent Boisselle (Gerrit)

unread,

11:15 AM (2 hours ago) 11:15 AM

to Jean-François Le, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org, tmartino+tran...@chromium.org

Attention needed from Jean-François Le

Vincent Boisselle added 1 comment

File ios/chrome/browser/autofill/model/autofill_java_script_feature_unittest.mm

Line 673, Patchset 3 (Latest): undoFirstFieldData.Set("isAutofilled", NO);

Vincent Boisselle . unresolved

curious, what happens when we undo autofills that are stacked ?

is `isAutofilled` going to be kept if we undo to another previous fill

Open in Gerrit

Related details

Attention is currently required from:

Jean-François Le

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[iOS] Undo autofill in javascript [chromium/src : main]

Jean-François Le (Gerrit)

Jean-François Le added 1 comment

Related details

Vincent Boisselle (Gerrit)

Vincent Boisselle added 5 comments

Related details

Jean-François Le (Gerrit)

Jean-François Le added 3 comments

Related details

Vincent Boisselle (Gerrit)

Vincent Boisselle added 1 comment

Related details

Jean-François Le (Gerrit)

Jean-François Le added 2 comments

Related details

Vincent Boisselle (Gerrit)

Vincent Boisselle added 1 comment

Related details