[autofill] Depending on profile order, sometimes merge/dedupe selects the wrong address |
|||||||||
Issue descriptionautofill_profile_comparator.cc contains a typo (or a copy-paste error) wherein it mistakenly keeps address1 instead of address2 when address2 is a superset of the tokens in address1. The same address->SetInfo() line is copied into both the address1 and address2 case.
,
Nov 30 2016
,
Nov 30 2016
Yes, and all other current channel branches.
,
Nov 30 2016
,
Nov 30 2016
The effect of this bug is that autofill profile merge/deduplication will always select the address from the first profile parameter passed to the merge function when one address is a subset of the other. The fix is a one character change to refer to the correct variable (address2) in the case where the second address should have been selected. The fix, including expanded unit tests, is already in trunk for M57, but as part of a larger CL [1]. I've created https://codereview.chromium.org/2535103004/ based on branch 2883 to apply the fix to M55 and for merge to M56. [1] https://codereview.chromium.org/2493253002
,
Nov 30 2016
Per chat with rogerm@, we will take this merge in for next M55 stable update. It won't be included in first M55 stable promotion.
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3cb0576de6902976d1b4271e0673aac0caa641fa commit 3cb0576de6902976d1b4271e0673aac0caa641fa Author: rogerm <rogerm@chromium.org> Date: Thu Dec 01 04:21:45 2016 [autofill] Merge can select the wrong address. Fixes a bug where, in the case where the the second address should have been retained during merge, the first address is retained, due to a typo in the referenced variable name. BUG= 669925 R=mathp@chromium.org NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2535103004 Cr-Commit-Position: refs/branch-heads/2883@{#700} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/3cb0576de6902976d1b4271e0673aac0caa641fa/components/autofill/core/browser/autofill_profile_comparator.cc
,
Dec 1 2016
Cl listed at #7 got merged to M55 without approval. This time it is ok as we already discussed about this. Next time, please wait for approval.
,
Dec 1 2016
Removing "Merge-Request-55" label as it is already merged to M55 at #7.
,
Dec 2 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8951973f96b7dba48c5cf3406fa0650beced8e5b commit 8951973f96b7dba48c5cf3406fa0650beced8e5b Author: Mathieu Perreault <mathp@chromium.org> Date: Fri Dec 02 18:27:48 2016 [Merge m56][autofill] Merge can select the wrong address. Fixes a bug where, in the case where the the second address should have been retained during merge, the first address is retained, due to a typo in the referenced variable name. BUG= 669925 R=mathp@chromium.org NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2535103004 Cr-Commit-Position: refs/branch-heads/2883@{#700} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} (cherry picked from commit 3cb0576de6902976d1b4271e0673aac0caa641fa) Review URL: https://codereview.chromium.org/2546653006 . Cr-Commit-Position: refs/branch-heads/2924@{#289} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/8951973f96b7dba48c5cf3406fa0650beced8e5b/components/autofill/core/browser/autofill_profile_comparator.cc
,
Dec 6 2016
Could you please provide the TEST steps if this can be verified manually.
,
Dec 6 2016
This is covered in unit-test coverage.
,
Dec 13 2016
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by zkoch@chromium.org
, Nov 30 2016