New issue
Advanced search Search tips

Issue 870370 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 27
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-08-06
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Remove the two toggle in Autofill settings page for M69

Project Member Reported by se...@chromium.org, Aug 2

Issue description

This will launch in M70 instead as it had not landed on all platforms.
 
This is what it looks like:
M69Settings.png
40.3 KB View Download
With the toggle set to off:
M69Settings_Disabled.png
40.3 KB View Download
Disabled by policy:
M69Settings_Policy.png
40.2 KB View Download
Labels: Merge-Request-69
Project Member

Comment 5 by sheriffbot@chromium.org, Aug 3

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The "split toggles" feature was landed on Desktop in M69 but the code for Android and iOS didn't make it.

For consistency, and to avoid having multiple risky merges to M69, it's safer to simply hide the toggles on Desktop for M69.
There is automated test coverage, and I manually tested it too
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #6, #7, #8. 
And per offline chat with sebsg@,
this need direct merge to M69. It got reviewed by all the relevant people though. The reason is that the code changed so much already in M70
that a merge would not be possible. It's only a change on the settings page.
NextAction: 2018-08-06
sebsg@ to merge to M69 on Monday morning.
Please merge your change to M69 branch 3497 by 4:00 PM PT, Monday (08/06) so we can pick it up for next week M69 beta release. Thank you.
The NextAction date has arrived: 2018-08-06
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 6

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/60c3e65bdca79ce2074eb6065536b4ad191f90ad

commit 60c3e65bdca79ce2074eb6065536b4ad191f90ad
Author: sebsg <sebsg@chromium.org>
Date: Mon Aug 06 13:14:37 2018

M69 [AF] Hide the settings toggles for addresses and cards.

Also always sets the prefs for enabling addresses and cards to true
unless explicitely turned off by a policy.

The split toggles will be launched in M70 instead.

Bug:  860526 ,  870370 
Change-Id: I335482fba1f7775b16b90cc78f6c3d830086d71a
Reviewed-on: https://chromium-review.googlesource.com/1160594
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#413}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/60c3e65bdca79ce2074eb6065536b4ad191f90ad/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
[modify] https://crrev.com/60c3e65bdca79ce2074eb6065536b4ad191f90ad/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js
[modify] https://crrev.com/60c3e65bdca79ce2074eb6065536b4ad191f90ad/chrome/test/data/webui/settings/settings_autofill_section_browsertest.js
[modify] https://crrev.com/60c3e65bdca79ce2074eb6065536b4ad191f90ad/components/autofill/core/browser/autofill_address_policy_handler.cc
[modify] https://crrev.com/60c3e65bdca79ce2074eb6065536b4ad191f90ad/components/autofill/core/browser/autofill_credit_card_policy_handler.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/abfc177db9f9eec401749069ca916520de36270d

commit abfc177db9f9eec401749069ca916520de36270d
Author: Moe Ahmadi <mahmadi@chromium.org>
Date: Thu Aug 23 16:31:56 2018

[AF][M69] Fixes policy unittests and borwsertests in M69

Fixes the following test failures in M69 caused by crrev.com/c/1160594.
PolicyPrefsTest.PolicyToPrefsMapping
AutofillAddressPolicyHandlerTest.Default
AutofillAddressPolicyHandlerTest.Enabled
AutofillCreditCardPolicyHandlerTest.Default
AutofillCreditCardPolicyHandlerTest.Enable

TBR=sebsg@ for components/autofill

Bug: 874535,874702, 870370 
Change-Id: If01d89b9fedeefbb91557d11908d773aedadc3bc
Reviewed-on: https://chromium-review.googlesource.com/1187083
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#788}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/abfc177db9f9eec401749069ca916520de36270d/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/abfc177db9f9eec401749069ca916520de36270d/components/autofill/core/browser/autofill_address_policy_handler_unittest.cc
[modify] https://crrev.com/abfc177db9f9eec401749069ca916520de36270d/components/autofill/core/browser/autofill_credit_card_policy_handler_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment