Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2432063003:
[Autofill] Offer credit card suggestions for some form action. (Closed)
|
Created:
4 years, 2 months ago by Mathieu Modified:
4 years, 2 months ago Reviewers:
sebsg CC:
chromium-reviews, rouslan+autofill_chromium.org, estade+watch_chromium.org, vabr+watchlistautofill_chromium.org, browser-components-watch_chromium.org, jdonnelly+autofillwatch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Autofill] Offer credit card suggestions for some form action.
A previous change had mistakenly filtered out some forms from showing
credit card suggestions. We now fix this and allow empty form actions
(which post to the current page) as well as "javascript:" handlers, for
example, all within pages that have a secure (and perhaps Mixed Passive) context.
BUG=652334
TEST=AutofillManagerTest,AutofillAssistantTest
Committed: https://crrev.com/87d76a1b9372afa7a29034936297c11c0eef8be1
Cr-Commit-Position: refs/heads/master@{#426915}
Patch Set 1 #Patch Set 2 : Fix Android tests #Patch Set 3 : rebase #Patch Set 4 : rebase tests #
Total comments: 2
Patch Set 5 : added a test #
Messages
Total messages: 30 (23 generated)
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest ========== to ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example, all within pages that have a secure context. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest ==========
Description was changed from ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example, all within pages that have a secure context. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest ========== to ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example, all within pages that have a secure (and perhaps Mixed Passive) context. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
mathp@chromium.org changed reviewers: + sebsg@chromium.org
Seb PTAL!
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/10/21 14:28:11, Mathieu Perreault wrote: > Seb PTAL! Bots are green now, sorry. PTAL
LGTM % comment :) https://codereview.chromium.org/2432063003/diff/60001/components/autofill/cor... File components/autofill/core/browser/autofill_assistant_unittest.cc (right): https://codereview.chromium.org/2432063003/diff/60001/components/autofill/cor... components/autofill/core/browser/autofill_assistant_unittest.cc:206: } Can you add one more test where the js function is invalid but the context is secure?
Thanks, will submit https://codereview.chromium.org/2432063003/diff/60001/components/autofill/cor... File components/autofill/core/browser/autofill_assistant_unittest.cc (right): https://codereview.chromium.org/2432063003/diff/60001/components/autofill/cor... components/autofill/core/browser/autofill_assistant_unittest.cc:206: } On 2016/10/21 19:10:55, sebsg wrote: > Can you add one more test where the js function is invalid but the context is > secure? Done.
The CQ bit was checked by mathp@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org Link to the patchset: https://codereview.chromium.org/2432063003/#ps80001 (title: "added a test")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example, all within pages that have a secure (and perhaps Mixed Passive) context. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest ========== to ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example, all within pages that have a secure (and perhaps Mixed Passive) context. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example, all within pages that have a secure (and perhaps Mixed Passive) context. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest ========== to ========== [Autofill] Offer credit card suggestions for some form action. A previous change had mistakenly filtered out some forms from showing credit card suggestions. We now fix this and allow empty form actions (which post to the current page) as well as "javascript:" handlers, for example, all within pages that have a secure (and perhaps Mixed Passive) context. BUG=652334 TEST=AutofillManagerTest,AutofillAssistantTest Committed: https://crrev.com/87d76a1b9372afa7a29034936297c11c0eef8be1 Cr-Commit-Position: refs/heads/master@{#426915} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/87d76a1b9372afa7a29034936297c11c0eef8be1 Cr-Commit-Position: refs/heads/master@{#426915} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
