ChromeOS: Time zone setting dropdown should be grayed out once we enable the Automatically resolve …

chrome/browser/resources/options/browser_options.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.exportPath('options');
 
 /**
  * @typedef {{actionLinkText: (string|undefined),
  *            childUser: (boolean|undefined),
  *            hasError: (boolean|undefined),
@@ skipping 91 matching lines @@
 
     /**
      * Track if page initialization is complete.  All C++ UI handlers have the
      * chance to manipulate page content within their InitializePage methods.
      * This flag is set to true after all initializers have been called.
      * @type {boolean}
      * @private
      */
     initializationComplete_: false,
 
+    /**
+     * Current status of "Resolve Timezone by Geolocation" checkbox.

[michaelpg, 2015/07/20 21:40:51]

Annotate: @private {boolean}

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

+     */
+    resolveTimezoneByGeolocation_: false,
+
+    /**
+     * True if system timezone is managed by policy.

[michaelpg, 2015/07/20 21:40:51]

Annotate.

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

+     */
+    systemTimezoneIsManaged_: false,
+
     /** @override */
     initializePage: function() {
       Page.prototype.initializePage.call(this);
       var self = this;
 
       if (window.top != window) {
         // The options page is not in its own window.
         document.body.classList.add('uber-frame');
         PageManager.horizontalOffset = 155;
       }
@@ skipping 278 matching lines @@
       }
 
       // Date and time section (CrOS only).
       if (cr.isChromeOS) {
         if ($('set-time-button'))
           $('set-time-button').onclick = this.handleSetTime_.bind(this);
 
         // Timezone
         if (loadTimeData.getBoolean('enableTimeZoneTrackingOption')) {
           $('resolve-timezone-by-geolocation-selection').hidden = false;
-          this.setSystemTimezoneManaged_(false);
-          $('timezone-value-select').disabled = loadTimeData.getBoolean(
-              'resolveTimezoneByGeolocationInitialValue');
         }
       }
 
       // Default browser section.
       if (!cr.isChromeOS) {
         if (!loadTimeData.getBoolean('showSetDefault')) {
           $('set-default-browser-section').hidden = true;
         }
         $('set-as-default-browser').onclick = function(event) {
           chrome.send('becomeDefaultBrowser');
@@ skipping 1222 matching lines @@
         this.enableElementIfPossible_(getRequiredElement('set-wallpaper'));
 
       // Create a synthetic pref change event decorated as
       // CoreOptionsHandler::CreateValueForPref() does.
       var event = new Event('wallpaper');
       event.value = managed ? { controlledBy: 'policy' } : {};
       $('wallpaper-indicator').handlePrefChange(event);
     },
 
     /**
-     * This is called from chromium code when system timezone "managed" state
-     * is changed. Enables or disables dependent settings.
-     * @param {boolean} managed Is true when system Timezone is managed by
-     *     enterprise policy. False otherwize.
+     * This enables or disables dependent settings in timezone section.
+     * @private
      */
-    setSystemTimezoneManaged_: function(managed) {
+    updateTimezoneSectionVisibility_: function() {

[michaelpg, 2015/07/20 21:40:51]

rename: this changes enabled state, not visibility

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

       if (loadTimeData.getBoolean('enableTimeZoneTrackingOption')) {
-        if (managed) {
+        if (this.systemTimezoneIsManaged_) {

[michaelpg, 2015/07/20 21:40:51]

If this is only used here, can you make it a parameter instead of a member
variable?

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

[michaelpg, 2015/07/21 23:47:36]

Sorry, I think my logic was wrong! There are 2 variables that can change
separately, and this function needs to know both of them, so what you had made
sense.

So this function doesn't need parameters after all. You can remove them (like
the code here) or leave them in (as they are in your latest patch), I don't care
much one way or the other.

           $('resolve-timezone-by-geolocation-selection').disabled = true;
           $('resolve-timezone-by-geolocation').onclick = function(event) {};
         } else {
           this.enableElementIfPossible_(
               getRequiredElement('resolve-timezone-by-geolocation-selection'));
           $('resolve-timezone-by-geolocation').onclick = function(event) {
             $('timezone-value-select').disabled = event.currentTarget.checked;
           };
           $('timezone-value-select').disabled =
-              $('resolve-timezone-by-geolocation').checked;
+              this.resolveTimezoneByGeolocation_;

[michaelpg, 2015/07/20 21:40:51]

Same request.

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

         }
       }
     },
 
     /**
+     * This is called from chromium code when system timezone "managed" state
+     * is changed. Enables or disables dependent settings.
+     * @param {boolean} managed Is true when system Timezone is managed by
+     *     enterprise policy. False otherwize.
+     */
+    setSystemTimezoneManaged_: function(managed) {
+      this.systemTimezoneIsManaged_ = managed;
+      this.updateTimezoneSectionVisibility_();
+    },
+
+    /**
+     * This is called from chromium code when kResolveTimezoneByGeolocation
+     * preference is changed. Enables or disables dependent settings.
+     * @param {boolean} value Is true or false when
+     *   kResolveTimezoneByGeolocation preference is true or false.

[michaelpg, 2015/07/20 21:40:51]

nit: indent 4 spaces

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

+     */
+    onResolveTimezoneByGeolocationChanged_: function(value) {

[michaelpg, 2015/07/20 21:40:51]

This is a plain preference, so you can trigger this function in JavaScript
instead of involving the C++ handler.

Preferences.getInstance().addEventListener(
    kResolveTimezoneByGeolocation,
    this.onResolveTimezoneByGeolocationChanged_.bind(this));

See preferences.js for definition and this file for examples.

[Alexander Alekseev, 2015/07/21 05:34:29]

This is what I was looking for! Thank you.

+      this.resolveTimezoneByGeolocation_ = value;
+      this.updateTimezoneSectionVisibility_();
+    },
+
+    /**
      * Handle the 'add device' button click.
      * @private
      */
     handleAddBluetoothDevice_: function() {
       chrome.send('coreOptionsUserMetricsAction',
                   ['Options_BluetoothShowAddDevice']);
       chrome.send('findBluetoothDevices');
       PageManager.showPageByName('bluetooth', false);
     },
 
@@ skipping 451 matching lines @@
   cr.makePublic(BrowserOptions, [
     'addBluetoothDevice',
     'deleteCurrentProfile',
     'enableCertificateButton',
     'enableDisplaySettings',
     'enableFactoryResetSection',
     'getCurrentProfile',
     'getStartStopSyncButton',
     'hideBluetoothSettings',
     'notifyInitializationComplete',
+    'onResolveTimezoneByGeolocationChanged',
     'removeBluetoothDevice',
     'scrollToSection',
     'setAccountPictureManaged',
     'setWallpaperManaged',
     'setAutoOpenFileTypesDisplayed',
     'setBluetoothState',
     'setCanSetTime',
     'setFontSize',
     'setHotwordRetrainLinkVisible',
     'setNativeThemeButtonEnabled',
@@ skipping 78 matching lines @@
       }
       button.textContent = loadTimeData.getString(strId);
     };
   }
 
   // Export
   return {
     BrowserOptions: BrowserOptions
   };
 });