Language settings: Fix Restart and Remove buttons shown at wrong times

chrome/browser/resources/settings/languages_page/languages.js

 // Copyright 2015 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.
 
 /**
  * @fileoverview 'settings-languages' handles Chrome's language and input
  * method settings. The 'languages' property, which reflects the current
  * language settings, must not be changed directly. Instead, changes to
  * language settings should be made using the LanguageHelper APIs provided by
  * this class via languageHelper.
@@ skipping 103 matching lines @@
     /**
      * Hash map of input methods supported for each language.
      * @type {!Map<string,
      *             !Array<!chrome.languageSettingsPrivate.InputMethod>>}
      * @private
      */
     languageInputMethods_: {
       type: Object,
       value: function() { return new Map(); },
     },
+
+    /** @private Prospective UI language when the page was loaded. */
+    originalProspectiveUILanguage_: String,
   },
 
   observers: [
+    'prospectiveUILanguageChanged_(prefs.intl.app_locale.value)',
     'preferredLanguagesPrefChanged_(' +
         'prefs.' + preferredLanguagesPrefName + '.value, languages)',
     'spellCheckDictionariesPrefChanged_(' +
         'prefs.spellcheck.dictionaries.value.*, languages)',
     'translateLanguagesPrefChanged_(' +
         'prefs.translate_blocked_languages.value.*, languages)',
     'updateRemovableLanguages_(' +
         'prefs.intl.app_locale.value, languages.enabled)',
     // Observe Chrome OS prefs (ignored for non-Chrome OS).
     'updateRemovableLanguages_(' +
@@ skipping 34 matching lines @@
           resolve(lists.componentExtensionImes.concat(
               lists.thirdPartyExtensionImes));
         });
       }.bind(this));
 
       promises[4] = new Promise(function(resolve) {
         this.inputMethodPrivate.getCurrentInputMethod(resolve);
       }.bind(this));
     }
 
+    if (cr.isWindows || cr.isChromeOS) {
+      // Fetch the starting UI language, which affects which actions should be
+      // enabled.
+      promises.push(cr.sendWithPromise('getProspectiveUILanguage').then(
+          function(prospectiveUILanguage) {
+            this.originalProspectiveUILanguage_ =
+                prospectiveUILanguage || navigator.language;

[stevenjb, 2016/12/13 18:53:09]

nit: I had to look up 'navigator.language', maybe use window.navigator.language?

[michaelpg, 2016/12/14 01:13:27]

Done.

+          }.bind(this)));
+    }
+
     Promise.all(promises).then(function(results) {
       this.createModel_(results[1], results[2], results[3], results[4]);
       this.resolver_.resolve();
     }.bind(this));
 
     if (cr.isChromeOS) {
       this.inputMethodPrivate.onChanged.addListener(
           this.onInputMethodChanged_.bind(this));
     }
   },
 
   /**
+   * Updates the prospective UI language based on the new pref value.
+   * @param {string} prospectiveUILanguage
+   * @private
+   */
+  prospectiveUILanguageChanged_: function(prospectiveUILanguage) {
+    if (!this.languages)
+      return;

[stevenjb, 2016/12/13 18:53:09]

Maybe make 'languages' an argument to this observer method so that a) we don't
need this check, and b) this gets called if this.languages gets set after
prospectiveUILanguage?

[michaelpg, 2016/12/14 01:13:27]

Done.

+
+    this.set('languages.prospectiveUILanguage',
+        prospectiveUILanguage || this.originalProspectiveUILanguage_);
+  },
+
+  /**
    * Updates the list of enabled languages from the preferred languages pref.
    * @private
    */
   preferredLanguagesPrefChanged_: function() {
-    var enabledLanguageStates =
-        this.getEnabledLanguageStates_(this.languages.translateTarget);
+    var enabledLanguageStates = this.getEnabledLanguageStates_(
+        this.languages.translateTarget, this.languages.prospectiveUILanguage);
 
     // Recreate the enabled language set before updating languages.enabled.
     this.enabledLanguageSet_.clear();
     for (var languageState of enabledLanguageStates)
       this.enabledLanguageSet_.add(languageState.language.code);
 
     this.set('languages.enabled', enabledLanguageStates);
   },
 
   /**
@@ skipping 10 matching lines @@
     }
   },
 
   /** @private */
   translateLanguagesPrefChanged_: function() {
     var translateBlockedPref = this.getPref('translate_blocked_languages');
     var translateBlockedSet = this.makeSetFromArray_(
         /** @type {!Array<string>} */(translateBlockedPref.value));
 
     for (var i = 0; i < this.languages.enabled.length; i++) {
-      if (this.languages.enabled[i].language.code == navigator.language)
+      if (this.languages.enabled[i].language.code ==
+          this.languages.prospectiveUILanguage) {
         continue;
+      }
       var translateCode = this.convertLanguageCodeForTranslate(
           this.languages.enabled[i].language.code);
       this.set(
           'languages.enabled.' + i + '.translateEnabled',
           !translateBlockedSet.has(translateCode));
     }
   },
 
   /**
    * Constructs the languages model.
@@ skipping 29 matching lines @@
           if (!this.supportedLanguageMap_.has(languageCode))
             continue;
           if (!this.languageInputMethods_.has(languageCode))
             this.languageInputMethods_.set(languageCode, [inputMethod]);
           else
             this.languageInputMethods_.get(languageCode).push(inputMethod);
         }
       }
     }
 
+    var prospectiveUILanguage;
+    if (cr.isChromeOS || cr.isWindows) {
+      prospectiveUILanguage =
+          /** @type {string} */(this.getPref('intl.app_locale').value) ||
+          this.originalProspectiveUILanguage_;
+    }
+
     // Create a list of enabled languages from the supported languages.
-    var enabledLanguageStates = this.getEnabledLanguageStates_(translateTarget);
+    var enabledLanguageStates =
+        this.getEnabledLanguageStates_(translateTarget, prospectiveUILanguage);
     // Populate the hash set of enabled languages.
     for (var languageState of enabledLanguageStates)
       this.enabledLanguageSet_.add(languageState.language.code);
 
     var model = /** @type {!LanguagesModel} */({
       supported: supportedLanguages,
       enabled: enabledLanguageStates,
       translateTarget: translateTarget,
     });
+
+    if (cr.isChromeOS || cr.isWindows)
+      model.prospectiveUILanguage = prospectiveUILanguage;
+
     if (cr.isChromeOS) {
       model.inputMethods = /** @type {!InputMethodsModel} */({
         supported: supportedInputMethods,
         enabled: this.getEnabledInputMethods_(),
         currentId: currentInputMethodId,
       });
     }
 
     // Initialize the Polymer languages model.
     this._setLanguages(model);
   },
 
   /**
    * Returns a list of LanguageStates for each enabled language in the supported
    * languages list.
    * @param {string} translateTarget Language code of the default translate
    *     target language.
+   * @param {(string|undefined)} prospectiveUILanguage Prospective UI display
+   *     language. Only defined on Windows and Chrome OS.
    * @return {!Array<!LanguageState>}
    * @private
    */
-  getEnabledLanguageStates_: function(translateTarget) {
+  getEnabledLanguageStates_: function(translateTarget, prospectiveUILanguage) {
     assert(CrSettingsPrefs.isInitialized);
 
     var pref = this.getPref(preferredLanguagesPrefName);
     var enabledLanguageCodes = pref.value.split(',');
     var spellCheckPref = this.getPref('spellcheck.dictionaries');
     var spellCheckSet = this.makeSetFromArray_(/** @type {!Array<string>} */(
         spellCheckPref.value));
 
     var translateBlockedPref = this.getPref('translate_blocked_languages');
     var translateBlockedSet = this.makeSetFromArray_(
         /** @type {!Array<string>} */(translateBlockedPref.value));
 
     var enabledLanguageStates = [];
     for (var i = 0; i < enabledLanguageCodes.length; i++) {
       var code = enabledLanguageCodes[i];
       var language = this.supportedLanguageMap_.get(code);
       // Skip unsupported languages.
       if (!language)
         continue;
       var languageState = /** @type {LanguageState} */({});
       languageState.language = language;
       languageState.spellCheckEnabled = !!spellCheckSet.has(code);
       // Translate is considered disabled if this language maps to any translate
       // language that is blocked.
       var translateCode = this.convertLanguageCodeForTranslate(code);
       languageState.translateEnabled = !!language.supportsTranslate &&
-          code != navigator.language &&
+          code != prospectiveUILanguage &&

[stevenjb, 2016/12/13 18:53:09]

This will be undefined on osx, right? Won't this always be true then? Maybe
that's OK, but I think it would be more clear if we made that explicit on non
win/cros.

[michaelpg, 2016/12/14 01:13:27]

Added !prospectiveUILanguage. There's already more cr.isWindows/cr.isChromeOS in
the code than I'd like...

[stevenjb, 2016/12/14 01:19:50]

Understood. It just wasn't clear that on platforms where prospectiveUILanguage
is undefined that this clause is expected to be true. It is more clear now.

           !translateBlockedSet.has(translateCode) &&
           translateCode != translateTarget;
       enabledLanguageStates.push(languageState);
     }
     return enabledLanguageStates;
   },
 
   /**
    * Returns a list of enabled input methods.
    * @return {!Array<!chrome.languageSettingsPrivate.InputMethod>}
@@ skipping 70 matching lines @@
   /** @return {!Promise} */
   whenReady: function() {
     return this.resolver_.promise;
   },
 
   /**
    * Sets the prospective UI language to the chosen language. This won't affect
    * the actual UI language until a restart.
    * @param {string} languageCode
    */
-  setUILanguage: function(languageCode) {
+  setProspectiveUILanguage: function(languageCode) {
     assert(cr.isChromeOS || cr.isWindows);
-    chrome.send('setUILanguage', [languageCode]);
-  },
-
-  /** Resets the prospective UI language back to the actual UI language. */
-  resetUILanguage: function() {
-    assert(cr.isChromeOS || cr.isWindows);
-    chrome.send('setUILanguage', [navigator.language]);
+    chrome.send('setProspectiveUILanguage', [languageCode]);
   },
 
   /**
-   * Returns the "prospective" UI language, i.e. the one to be used on next
-   * restart. If the pref is not set, the current UI language is also the
-   * "prospective" language.
-   * @return {string} Language code of the prospective UI language.
+   * True if the prospective UI language was changed from its starting value.
+   * @return {boolean}
    */
-  getProspectiveUILanguage: function() {
-    return /** @type {string} */(this.getPref('intl.app_locale').value) ||
-        navigator.language;
+  requiresRestart: function() {
+    return this.originalProspectiveUILanguage_ !=
+        this.languages.prospectiveUILanguage;
   },
 
   /**
    * @param {string} languageCode
    * @return {boolean} True if the language is enabled.
    */
   isLanguageEnabled: function(languageCode) {
     return this.enabledLanguageSet_.has(languageCode);
   },
 
@@ skipping 40 matching lines @@
     this.languageSettingsPrivate.disableLanguage(languageCode);
     this.enableTranslateLanguage(languageCode);
   },
 
   /**
    * @param {string} languageCode Language code for an enabled language.
    * @return {boolean}
    */
   canDisableLanguage: function(languageCode) {
     // Cannot disable the prospective UI language.
-    if (languageCode == this.getProspectiveUILanguage())
+    if (languageCode == this.languages.prospectiveUILanguage)
       return false;
 
     // Cannot disable the only enabled language.
     if (this.languages.enabled.length == 1)
       return false;
 
     if (!cr.isChromeOS)
       return true;
 
     // If this is the only enabled language that is supported by all enabled
@@ skipping 217 matching lines @@
     this.updateEnabledInputMethods_();
   },
 
   /** @param {string} id Removed input method ID. */
   onInputMethodRemoved_: function(id) {
     assert(cr.isChromeOS);
     this.updateEnabledInputMethods_();
   },
 });
 })();