Issue metadata
Sign in to add a comment
|
Login username not filled on kakao.com |
||||||||||||||||||||||
Issue descriptionChrome Version: 72.0.3583.0 OS: GNU/Linux What steps will reproduce the problem? (1) Go to https://accounts.kakao.com (2) Save some credentials on that login form (3) Visit https://accounts.kakao.com again What is the expected result? Credentials being autofilled What happens instead? No automatic filling. Filling on demand only affects the password field. Despite the server correctly finds the username (SERVER_PREDICTION: USERNAME for the field named 'email'), the new parser somehow ends up choosing a 'phone_number' (through HTML classifier) field as username. This field is readonly and not visible. For being readonly, PasswordAutofillAgent::FillUserNameAndPassword refuses to fill.
,
Oct 16
So this is actually working as intended so far: Initially, we wanted to avoid mixing results from various types of analysis. So if the server-side analysis finds the username but not the password, it is scrapped and both finding the username and the password is left to other types of analysis. We already broke this principle in issue 854130 , where we allowed the base heuristics/HTML classifier to add a username to a password-only result from server or autocomplete attribute. Here the opposite helps: adding the server hint about username to the base heuristic about passwords. Or, we might make the server also predict the password, in which case the parser will follow the hint for both the password and username.
,
Oct 16
Ah, and an explanation why the old parser chose the correct username, even without heuristics: The old parser first discards all fields with the "disabled" attribute. Then, it looks at HTML classifier list. The list in this case was a singleton with the wrong username candidate, so it was ignored. Basic heuristics then picked the nearest enabled field before the password, which was the correct one. The new parser makes no difference between disabled and enabled field, as justified in the design doc [1]. Therefore it accepted the HTML classifier result and went with the wrong username. My opinion is that the reasons to make no difference between disabled and enabled pertain, and we should pick one of the solutions suggested in #2. [1] https://docs.google.com/document/d/1KxWnt3-Pykz4ut4P1IHxNhn6Ef64v3VMyGobUWyLnDg/edit#heading=h.azxb6xacxjq2
,
Oct 29
Given that this used to work in the old parser, shouldn't this be marked as type=regression?
,
Oct 30
Well, most of the bugs appeared at one point and weren't there before. But sure, why not mark it as regression. Does it change anything?
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0d257042461e7503f9ac798a1e6377a2c41254c1 commit 0d257042461e7503f9ac798a1e6377a2c41254c1 Author: Vadym Doroshenko <dvadym@chromium.org> Date: Wed Nov 07 14:45:40 2018 Fix dynamically changed forms in NewPasswordFormManager. Sometimes sites change forms after adding them. For example removing/ adding fields. This CL makes NewPasswordFormManager process this situation, by updating |observed_form|, parsing it again and sending new data to the renderer. Bug: 895411, 831123, 895788 Change-Id: I296180c5b30f879a3f9bcc7af7cccb98f5fa8eb8 Reviewed-on: https://chromium-review.googlesource.com/c/1317893 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#606041} [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/components/password_manager/core/browser/new_password_form_manager.cc [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/components/password_manager/core/browser/new_password_form_manager.h [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/components/password_manager/core/browser/new_password_form_manager_unittest.cc [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/components/password_manager/core/browser/password_form_metrics_recorder.cc [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/components/password_manager/core/browser/password_form_metrics_recorder.h [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/components/password_manager/core/browser/password_manager.cc [modify] https://crrev.com/0d257042461e7503f9ac798a1e6377a2c41254c1/tools/metrics/histograms/histograms.xml
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1be2ac8dac1222f9827b32455f7f517cfcadea0d commit 1be2ac8dac1222f9827b32455f7f517cfcadea0d Author: Vaclav Brozek <vabr@chromium.org> Date: Wed Nov 07 19:29:32 2018 Revert "Fix dynamically changed forms in NewPasswordFormManager." This reverts commit 0d257042461e7503f9ac798a1e6377a2c41254c1. Reason for revert: When I ran out/Default/interactive_ui_tests --gtest_filter=PasswordGenerationInteractiveTest.PopupShownAutomaticallyAndPasswordErased --gtest_repeat=10 --gtest_break_on_failure with the patch in, on my (virtual) Linux machine, I could reproduce the test failure reliably. When I reverted the patch locally, the test always succeeded. Original change's description: > Fix dynamically changed forms in NewPasswordFormManager. > > Sometimes sites change forms after adding them. For example removing/ > adding fields. This CL makes NewPasswordFormManager process this > situation, by updating |observed_form|, parsing it again and sending > new data to the renderer. > > Bug: 895411, 831123, 895788 > Change-Id: I296180c5b30f879a3f9bcc7af7cccb98f5fa8eb8 > Reviewed-on: https://chromium-review.googlesource.com/c/1317893 > Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> > Reviewed-by: Ilya Sherman <isherman@chromium.org> > Reviewed-by: Vaclav Brozek <vabr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#606041} TBR=isherman@chromium.org,vabr@chromium.org,dvadym@chromium.org Change-Id: I3c1f2e4570bd07b9cba0f0bdbbe65d7dc28300a9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 895411, 831123, 895788 Reviewed-on: https://chromium-review.googlesource.com/c/1323657 Reviewed-by: Vaclav Brozek <vabr@chromium.org> Commit-Queue: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#606126} [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/components/password_manager/core/browser/new_password_form_manager.cc [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/components/password_manager/core/browser/new_password_form_manager.h [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/components/password_manager/core/browser/new_password_form_manager_unittest.cc [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/components/password_manager/core/browser/password_form_metrics_recorder.cc [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/components/password_manager/core/browser/password_form_metrics_recorder.h [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/components/password_manager/core/browser/password_manager.cc [modify] https://crrev.com/1be2ac8dac1222f9827b32455f7f517cfcadea0d/tools/metrics/histograms/histograms.xml
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b874f30df5bdd328cd3cf5d52c1c623b11f45fec commit b874f30df5bdd328cd3cf5d52c1c623b11f45fec Author: Vadym Doroshenko <dvadym@chromium.org> Date: Thu Nov 08 13:07:26 2018 Fix dynamically changed forms in NewPasswordFormManager. Sometimes sites change forms after adding them. For example removing/ adding fields. This CL makes NewPasswordFormManager process this situation, by updating |observed_form|, parsing it again and sending new data to the renderer. This first time this CL was uploaded on https://chromium-review.googlesource.com/c/chromium/src/+/1326010 (Patch 1) but it was reverted because of failures in password_generation_interactive_uitest.cc. The reason of failures is that no non-blacklisted confirmation because NewPasswordFormManager waits for server response. This CL fixes that by turning off waiting of server predictions. TBR=isherman@chromium.org Bug: 895411, 831123, 895788 , 902805 Change-Id: I99174a73134c582a3975b1f06d797e61ec117855 Reviewed-on: https://chromium-review.googlesource.com/c/1326010 Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Cr-Commit-Position: refs/heads/master@{#606435} [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/chrome/browser/password_manager/password_generation_interactive_uitest.cc [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/components/password_manager/core/browser/new_password_form_manager.cc [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/components/password_manager/core/browser/new_password_form_manager.h [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/components/password_manager/core/browser/new_password_form_manager_unittest.cc [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/components/password_manager/core/browser/password_form_metrics_recorder.cc [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/components/password_manager/core/browser/password_form_metrics_recorder.h [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/components/password_manager/core/browser/password_manager.cc [modify] https://crrev.com/b874f30df5bdd328cd3cf5d52c1c623b11f45fec/tools/metrics/histograms/histograms.xml
,
Nov 12
The issue seems fixed, strangely even before r606435. I tried to bisect to understand what fixed it, but bisecting failed (bug 904347).
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ddbedba5c0d59f895623bfec8d548f1c70086baf commit ddbedba5c0d59f895623bfec8d548f1c70086baf Author: Vaclav Brozek <vabr@chromium.org> Date: Mon Nov 12 18:21:38 2018 [M71] Fix dynamically changed forms in NewPasswordFormManager. Sometimes sites change forms after adding them. For example removing/ adding fields. This CL makes NewPasswordFormManager process this situation, by updating |observed_form|, parsing it again and sending new data to the renderer. TBR=dvadym@chromium.org (cherry picked from commit b874f30df5bdd328cd3cf5d52c1c623b11f45fec) Bug: 895411, 831123, 895788 Change-Id: I99174a73134c582a3975b1f06d797e61ec117855 Reviewed-on: https://chromium-review.googlesource.com/c/1326010 Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#606435} Reviewed-on: https://chromium-review.googlesource.com/c/1331590 Cr-Commit-Position: refs/branch-heads/3578@{#636} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/chrome/browser/password_manager/password_generation_interactive_uitest.cc [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/components/password_manager/core/browser/new_password_form_manager.cc [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/components/password_manager/core/browser/new_password_form_manager.h [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/components/password_manager/core/browser/new_password_form_manager_unittest.cc [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/components/password_manager/core/browser/password_form_metrics_recorder.cc [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/components/password_manager/core/browser/password_form_metrics_recorder.h [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/components/password_manager/core/browser/password_manager.cc [modify] https://crrev.com/ddbedba5c0d59f895623bfec8d548f1c70086baf/tools/metrics/histograms/histograms.xml
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ddbedba5c0d59f895623bfec8d548f1c70086baf Commit: ddbedba5c0d59f895623bfec8d548f1c70086baf Author: vabr@chromium.org Commiter: vabr@chromium.org Date: 2018-11-12 18:21:38 +0000 UTC [M71] Fix dynamically changed forms in NewPasswordFormManager. Sometimes sites change forms after adding them. For example removing/ adding fields. This CL makes NewPasswordFormManager process this situation, by updating |observed_form|, parsing it again and sending new data to the renderer. TBR=dvadym@chromium.org (cherry picked from commit b874f30df5bdd328cd3cf5d52c1c623b11f45fec) Bug: 895411, 831123, 895788 Change-Id: I99174a73134c582a3975b1f06d797e61ec117855 Reviewed-on: https://chromium-review.googlesource.com/c/1326010 Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#606435} Reviewed-on: https://chromium-review.googlesource.com/c/1331590 Cr-Commit-Position: refs/branch-heads/3578@{#636} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4 commit cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4 Author: Olivier Robin <olivierrobin@chromium.org> Date: Tue Nov 13 09:52:03 2018 Revert "[M71] Fix dynamically changed forms in NewPasswordFormManager." This reverts commit ddbedba5c0d59f895623bfec8d548f1c70086baf. Reason for revert: Break M71 compilation. base::HistogramTester does not exist on M71 Original change's description: > [M71] Fix dynamically changed forms in NewPasswordFormManager. > > Sometimes sites change forms after adding them. For example removing/ > adding fields. This CL makes NewPasswordFormManager process this > situation, by updating |observed_form|, parsing it again and sending > new data to the renderer. > > TBR=dvadym@chromium.org > > (cherry picked from commit b874f30df5bdd328cd3cf5d52c1c623b11f45fec) > > Bug: 895411, 831123, 895788 > Change-Id: I99174a73134c582a3975b1f06d797e61ec117855 > Reviewed-on: https://chromium-review.googlesource.com/c/1326010 > Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> > Reviewed-by: Vaclav Brozek <vabr@chromium.org> > Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#606435} > Reviewed-on: https://chromium-review.googlesource.com/c/1331590 > Cr-Commit-Position: refs/branch-heads/3578@{#636} > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} TBR=vabr@chromium.org,dvadym@chromium.org Change-Id: Ib55a6643c0d9a5a88f744bf8cb4e650bdb9755a1 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 895411, 831123, 895788 Reviewed-on: https://chromium-review.googlesource.com/c/1332195 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#652} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/chrome/browser/password_manager/password_generation_interactive_uitest.cc [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/components/password_manager/core/browser/new_password_form_manager.cc [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/components/password_manager/core/browser/new_password_form_manager.h [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/components/password_manager/core/browser/new_password_form_manager_unittest.cc [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/components/password_manager/core/browser/password_form_metrics_recorder.cc [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/components/password_manager/core/browser/password_form_metrics_recorder.h [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/components/password_manager/core/browser/password_manager.cc [modify] https://crrev.com/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4/tools/metrics/histograms/histograms.xml
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4 Commit: cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4 Author: olivierrobin@chromium.org Commiter: olivierrobin@chromium.org Date: 2018-11-13 09:52:03 +0000 UTC Revert "[M71] Fix dynamically changed forms in NewPasswordFormManager." This reverts commit ddbedba5c0d59f895623bfec8d548f1c70086baf. Reason for revert: Break M71 compilation. base::HistogramTester does not exist on M71 Original change's description: > [M71] Fix dynamically changed forms in NewPasswordFormManager. > > Sometimes sites change forms after adding them. For example removing/ > adding fields. This CL makes NewPasswordFormManager process this > situation, by updating |observed_form|, parsing it again and sending > new data to the renderer. > > TBR=dvadym@chromium.org > > (cherry picked from commit b874f30df5bdd328cd3cf5d52c1c623b11f45fec) > > Bug: 895411, 831123, 895788 > Change-Id: I99174a73134c582a3975b1f06d797e61ec117855 > Reviewed-on: https://chromium-review.googlesource.com/c/1326010 > Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> > Reviewed-by: Vaclav Brozek <vabr@chromium.org> > Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#606435} > Reviewed-on: https://chromium-review.googlesource.com/c/1331590 > Cr-Commit-Position: refs/branch-heads/3578@{#636} > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} TBR=vabr@chromium.org,dvadym@chromium.org Change-Id: Ib55a6643c0d9a5a88f744bf8cb4e650bdb9755a1 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 895411, 831123, 895788 Reviewed-on: https://chromium-review.googlesource.com/c/1332195 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#652} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 13
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf3759a73437677de98b3894acf517981508b987 commit bf3759a73437677de98b3894acf517981508b987 Author: Vaclav Brozek <vabr@chromium.org> Date: Tue Nov 13 10:14:28 2018 Reland "[M71] Fix dynamically changed forms in NewPasswordFormManager." This reverts commit cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4. Reason for revert: The breakage was caused by not #including "base/test/metrics/histogram_tester.h" in new_password_form_manager_unittest.cc directly. The missing #include does exist on trunk. Also, the file gets #included indirectly on desktop platforms, so this was not caught by go/betabuilders. Patchset 1 here is the reverted CL, patchset 2 is the fixed one. Original change's description: > Revert "[M71] Fix dynamically changed forms in NewPasswordFormManager." > > This reverts commit ddbedba5c0d59f895623bfec8d548f1c70086baf. > > Reason for revert: Break M71 compilation. > base::HistogramTester does not exist on M71 > > Original change's description: > > [M71] Fix dynamically changed forms in NewPasswordFormManager. > > > > Sometimes sites change forms after adding them. For example removing/ > > adding fields. This CL makes NewPasswordFormManager process this > > situation, by updating |observed_form|, parsing it again and sending > > new data to the renderer. > > > > TBR=dvadym@chromium.org > > > > (cherry picked from commit b874f30df5bdd328cd3cf5d52c1c623b11f45fec) > > > > Bug: 895411, 831123, 895788 > > Change-Id: I99174a73134c582a3975b1f06d797e61ec117855 > > Reviewed-on: https://chromium-review.googlesource.com/c/1326010 > > Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> > > Reviewed-by: Vaclav Brozek <vabr@chromium.org> > > Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> > > Cr-Original-Commit-Position: refs/heads/master@{#606435} > > Reviewed-on: https://chromium-review.googlesource.com/c/1331590 > > Cr-Commit-Position: refs/branch-heads/3578@{#636} > > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} > > TBR=vabr@chromium.org,dvadym@chromium.org > > Change-Id: Ib55a6643c0d9a5a88f744bf8cb4e650bdb9755a1 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 895411, 831123, 895788 > Reviewed-on: https://chromium-review.googlesource.com/c/1332195 > Reviewed-by: Olivier Robin <olivierrobin@chromium.org> > Cr-Commit-Position: refs/branch-heads/3578@{#652} > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} TBR=vabr@chromium.org,dvadym@chromium.org,olivierrobin@chromium.org Change-Id: Ic698b6e38a79b897f9fc9fda2676ee7a22112113 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 895411, 831123, 895788 Reviewed-on: https://chromium-review.googlesource.com/c/1333367 Reviewed-by: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#654} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/chrome/browser/password_manager/password_generation_interactive_uitest.cc [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/components/password_manager/core/browser/new_password_form_manager.cc [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/components/password_manager/core/browser/new_password_form_manager.h [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/components/password_manager/core/browser/new_password_form_manager_unittest.cc [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/components/password_manager/core/browser/password_form_metrics_recorder.cc [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/components/password_manager/core/browser/password_form_metrics_recorder.h [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/components/password_manager/core/browser/password_manager.cc [modify] https://crrev.com/bf3759a73437677de98b3894acf517981508b987/tools/metrics/histograms/histograms.xml
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf3759a73437677de98b3894acf517981508b987 Commit: bf3759a73437677de98b3894acf517981508b987 Author: vabr@chromium.org Commiter: vabr@chromium.org Date: 2018-11-13 10:14:28 +0000 UTC Reland "[M71] Fix dynamically changed forms in NewPasswordFormManager." This reverts commit cbf87871a1ad26f4fe6b9efaf33c9c6181e04cb4. Reason for revert: The breakage was caused by not #including "base/test/metrics/histogram_tester.h" in new_password_form_manager_unittest.cc directly. The missing #include does exist on trunk. Also, the file gets #included indirectly on desktop platforms, so this was not caught by go/betabuilders. Patchset 1 here is the reverted CL, patchset 2 is the fixed one. Original change's description: > Revert "[M71] Fix dynamically changed forms in NewPasswordFormManager." > > This reverts commit ddbedba5c0d59f895623bfec8d548f1c70086baf. > > Reason for revert: Break M71 compilation. > base::HistogramTester does not exist on M71 > > Original change's description: > > [M71] Fix dynamically changed forms in NewPasswordFormManager. > > > > Sometimes sites change forms after adding them. For example removing/ > > adding fields. This CL makes NewPasswordFormManager process this > > situation, by updating |observed_form|, parsing it again and sending > > new data to the renderer. > > > > TBR=dvadym@chromium.org > > > > (cherry picked from commit b874f30df5bdd328cd3cf5d52c1c623b11f45fec) > > > > Bug: 895411, 831123, 895788 > > Change-Id: I99174a73134c582a3975b1f06d797e61ec117855 > > Reviewed-on: https://chromium-review.googlesource.com/c/1326010 > > Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> > > Reviewed-by: Vaclav Brozek <vabr@chromium.org> > > Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> > > Cr-Original-Commit-Position: refs/heads/master@{#606435} > > Reviewed-on: https://chromium-review.googlesource.com/c/1331590 > > Cr-Commit-Position: refs/branch-heads/3578@{#636} > > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} > > TBR=vabr@chromium.org,dvadym@chromium.org > > Change-Id: Ib55a6643c0d9a5a88f744bf8cb4e650bdb9755a1 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 895411, 831123, 895788 > Reviewed-on: https://chromium-review.googlesource.com/c/1332195 > Reviewed-by: Olivier Robin <olivierrobin@chromium.org> > Cr-Commit-Position: refs/branch-heads/3578@{#652} > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} TBR=vabr@chromium.org,dvadym@chromium.org,olivierrobin@chromium.org Change-Id: Ic698b6e38a79b897f9fc9fda2676ee7a22112113 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 895411, 831123, 895788 Reviewed-on: https://chromium-review.googlesource.com/c/1333367 Reviewed-by: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#654} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 29
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by vabr@chromium.org
, Oct 16