New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 894598 link

Starred by 1 user
Status: Fixed
Owner:
█ jhawkins@chromium.org
Last visit > 30 days ago
Closed: Oct 22
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Properly handle enterprise controlled sign-in policy in Settings

Project Member Reported by jhawkins@chromium.org, Oct 11 
Currently the user may toggle the sign-in enabled pref in the Smart Lock Settings even if the enterprise policy disallows this part of the feature.  This does not affect whether the policy is respected or not, but disabling the radio control and showing that the setting is enterprise-controlled is important.

Comment 1 by jhawkins@chromium.org, Oct 16

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 18 

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

commit 0b3eda72f5cf23bd2c78368de0a584ce3466e2f8
Author: James Hawkins <jhawkins@chromium.org>
Date: Thu Oct 18 16:55:48 2018

Smart Lock: Display the enterprise controlled state of sign-in pref.

R=khorimoto@chromium.org

Bug:  894598 
Test: none
Change-Id: I85bfe11dda29295d45c609851222bd40fff98ef7
Reviewed-on: https://chromium-review.googlesource.com/c/1283486
Commit-Queue: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600800}
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/resources/settings/multidevice_page/BUILD.gn
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/resources/settings/multidevice_page/multidevice_browser_proxy.js
[add] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/resources/settings/multidevice_page/multidevice_radio_button.html
[add] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/resources/settings/multidevice_page/multidevice_radio_button.js
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/resources/settings/multidevice_page/multidevice_smartlock_subpage.html
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/resources/settings/multidevice_page/multidevice_smartlock_subpage.js
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/resources/settings/settings_resources.grd
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.cc
[modify] https://crrev.com/0b3eda72f5cf23bd2c78368de0a584ce3466e2f8/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h

Comment 3 by hsuregan@google.com, Oct 18

Labels: -Pri-2 Merge-Request-71 Pri-1
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 19

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: There is .grd file changes and we are only 45 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by kbleicher@google.com, Oct 19 

His is a fairly long list of changes and I'm not catching comments on how well this has been tested.  Need some confidence re: risk prior to merge approval.  Thanks.

Comment 6 by jhawkins@chromium.org, Oct 22 

kbleicher: Has been tested thoroughly, and the change is purely UI.  I think the risk is low since this is making use of existing code that is highly used and well-tested.

Comment 7 by kbleicher@google.com, Oct 22

Labels: -Merge-Review-71 Merge-Approved-71
Thanks for the update.
We're also supposed to freeze string translation post branch.  Is this an emergency change, and/or are these applicable for translation?

Comment 8 by jhawkins@chromium.org, Oct 22 

This CL doesn't have any string changes (the grd change is just for files).
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 22

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd

commit 78430a84ec42d4ce02d5f9d1e6193a7135e82dcd
Author: James Hawkins <jhawkins@chromium.org>
Date: Mon Oct 22 20:33:50 2018

Smart Lock: Display the enterprise controlled state of sign-in pref.

R=​khorimoto@chromium.org

Bug:  894598 
Test: none
Change-Id: I85bfe11dda29295d45c609851222bd40fff98ef7
Reviewed-on: https://chromium-review.googlesource.com/c/1283486
Commit-Queue: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600800}(cherry picked from commit 0b3eda72f5cf23bd2c78368de0a584ce3466e2f8)
Reviewed-on: https://chromium-review.googlesource.com/c/1294672
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#243}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/resources/settings/multidevice_page/BUILD.gn
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/resources/settings/multidevice_page/multidevice_browser_proxy.js
[add] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/resources/settings/multidevice_page/multidevice_radio_button.html
[add] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/resources/settings/multidevice_page/multidevice_radio_button.js
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/resources/settings/multidevice_page/multidevice_smartlock_subpage.html
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/resources/settings/multidevice_page/multidevice_smartlock_subpage.js
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/resources/settings/settings_resources.grd
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.cc
[modify] https://crrev.com/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h

Comment 10 by jhawkins@chromium.org, Oct 22

Labels: -Hotlist-Merge-Review
Status: Fixed (was: Started)
Project Member

Comment 11 by cr-audit...@appspot.gserviceaccount.com, Oct 23

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/78430a84ec42d4ce02d5f9d1e6193a7135e82dcd

Commit: 78430a84ec42d4ce02d5f9d1e6193a7135e82dcd
Author: jhawkins@chromium.org
Commiter: jhawkins@chromium.org
Date: 2018-10-22 20:33:50 +0000 UTC

Smart Lock: Display the enterprise controlled state of sign-in pref.

R=​khorimoto@chromium.org

Bug:  894598 
Test: none
Change-Id: I85bfe11dda29295d45c609851222bd40fff98ef7
Reviewed-on: https://chromium-review.googlesource.com/c/1283486
Commit-Queue: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600800}(cherry picked from commit 0b3eda72f5cf23bd2c78368de0a584ce3466e2f8)
Reviewed-on: https://chromium-review.googlesource.com/c/1294672
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#243}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
►

Sign in to add a comment