MD Settings: Languages model for language pages

chrome/browser/resources/settings/languages_page/languages_model.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 'cr-settings-languages-model' provides convenient access to
+ * Chrome's language and input method settings.
+ *
+ * @group Chrome Settings Elements
+ * @element cr-settings-languages-model
+ */
+
+/**
+ * @typedef {{spellCheckEnabled: boolean}}
+ */
+var LanguageState;
+
+/**
+ * @typedef {{language: !chrome.languageSettingsPrivate.Language,
+ *            state: !LanguageState}}
+ */
+var LanguageInfo;
+
+/**
+ * The model includes:
+ *     supportedLanguages: an array of languages, ordered alphabetically.
+ *     enabledLanguages: an array of enabled language info and state, ordered by
+ *         preference.
+ * @typedef {{
+ *      supportedLanguages: !Array<!chrome.languageSettingsPrivate.Language>,
+ *      enabledLanguages: !Array<!LanguageInfo>
+ *  }}
+ */
+var LanguagesModel;
+
+(function() {
+'use strict';
+
+/** Exposes the singleton model to hosts. */
+Polymer({
+  is: 'cr-settings-languages-model',
+
+  properties: {
+    /**
+     * Shared languages model.
+     * @type {(LanguagesModel|undefined)}
+     */
+    model: {
+      type: Object,
+      notify: true,
+      readOnly: true,
+    },
+
+    /**
+     * Shared private state.

[stevenjb, 2015/09/22 01:45:15]

nit: This only kind of makes sense to me because we've talked about it some. I
think that a comment or reference to a document describing how this works would
be helpful.

[michaelpg, 2015/09/22 04:03:15]

I've added documentation in the file overview and expanded some of the comments.

+     * @type {!Element}
+     */
+    privateModel_: {
+      type: Object,
+      value: document.createElement('cr-settings-languages-model-private'),
+    },
+  },
+
+  ready: function() {
+    this.squelching_(function() {
+      // This notifies the host via Polymer.
+      this._setModel(this.privateModel_.model);
+    });
+    this.listen(this.privateModel_, 'model-changed', 'privateModelChanged_');
+  },
+
+  /**
+   * On supported systems, sets the prospective UI language to the chosen
+   * language. This dosen't affect the actual UI language until a restart.

[stevenjb, 2015/09/22 01:45:15]

Rather than duplicating these comments, I would either comment each with
something like '* Forwarding function' or document the whole section since it is
pretty clear what these are doing.

[michaelpg, 2015/09/22 04:03:15]

Done.

+   * @param {string} languageCode
+   */
+  setUILanguage: function(languageCode) {
+    this.privateModel_.setUILanguage(languageCode);
+  },
+
+  /**
+   * Enables the language, making it available for spell check and input.
+   * @param {string} languageCode
+   * */
+  enableLanguage: function(languageCode) {
+    this.privateModel_.enableLanguage(languageCode);
+  },
+
+  /**
+   * Disables the language.
+   * @param {string} languageCode
+   */
+  disableLanguage: function(languageCode) {
+    this.privateModel_.disableLanguage(languageCode);
+  },
+
+  /**
+   * @param {string} languageCode
+   * @return {boolean} True if the language is enabled.
+   */
+  isEnabled: function(languageCode) {
+    return this.privateModel_.isEnabled(languageCode);
+  },
+
+  /**
+   * Enables or disables spell check for the given language.
+   * @param {string} languageCode
+   * @param {boolean} enable
+   */
+  toggleSpellCheck: function(languageCode, enable) {
+    this.privateModel_.toggleSpellCheck(languageCode, enable);
+  },
+
+  /**
+   * Forwards changes reported by privateModel_ to the host.
+   * @private
+   */
+  privateModelChanged_: function(e) {
+    // Squelch because we've defeated Polymer's internal dirty-checking.
+    this.squelching_(function() {
+      // Forward notification to host.
+      this.fire(e.type, e.detail, {bubbles: false});
+    });
+  },
+
+  /**
+   * Sets a "squelch" switch before calling the function, so functions can
+   * return early when the switch is active.
+   * @param {!function()} fn
+   * @private
+   */
+  squelching_: function(fn) {
+    this.squelch_ = true;

[stevenjb, 2015/09/22 01:45:15]

I don't see anywhere that we check |squelch_| ?

[michaelpg, 2015/09/22 04:03:15]

You're right, we don't need to because model is readOnly: we don't have to
propagate changes *to* the singleton, so we're not in danger of repeating
events. Done.

+    fn.call(this);
+    // We can unset squelch_ now because change notifications are synchronous.
+    this.squelch_ = false;
+  },
+});
+
+var preferredLanguagesPath;
+if (cr.isChromeOS)
+  preferredLanguagesPath = 'prefs.settings.language.preferred_languages.value';
+else
+  preferredLanguagesPath = 'prefs.intl.accept_languages.value';
+
+/**
+ * Privately used element that contains, listens to and updates the shared
+ * languages model.
+ */
+Polymer({
+  is: 'cr-settings-languages-model-private',
+
+  properties: {
+    /**
+     * @type {(LanguagesModel|undefined)}
+     */
+    model: {
+      type: Object,
+      notify: true,
+    },
+
+    /**
+     * Object containing all preferences, for use by Polymer controls.
+     */
+    prefs: {
+      type: Object,
+      notify: true,
+    },
+  },
+
+  /**
+   * Hash map of model.supportedLanguages for fast lookup. Not data bound.

[stevenjb, 2015/09/22 01:45:16]

nit: describe what the key is for this map and the one below (language code I
assume?)

[michaelpg, 2015/09/22 04:03:15]

Done.

+   * @private {!Object<!chrome.languageSettingsPrivate.Language>}
+   */
+  supportedLanguageMap_: {},
+
+  /**
+   * Hash maps of model.enabledLanguages for fast lookup. Not data bound.
+   * @private {!Object<!LanguageInfo>}
+   */
+  enabledLanguageMap_: {},
+
+  observers: [
+    'preferredLanguagesPrefChanged_(' + preferredLanguagesPath + ')',
+    'spellCheckDictionariesPrefChanged_(prefs.spellcheck.dictionaries.value.*)',
+  ],
+
+  /** @override */
+  created: function() {
+    chrome.languageSettingsPrivate.getLanguageList(function(languageList) {
+      // Wait until prefs are initialized before creating the model, so we can
+      // include information about enabled languages.
+      CrSettingsPrefs.initialized.then(function() {
+        this.createModel_(languageList);
+        this.initialized_ = true;
+      }.bind(this));
+    }.bind(this));
+  },
+
+  /**
+   * Constructs the model with the given language list.
+   * @param {!Array<!chrome.languageSettingsPrivate.Language>}
+   *     supportedLanguages
+   */
+  createModel_: function(supportedLanguages) {
+    // Populate the hash map of supported languages.
+    for (var i = 0; i < supportedLanguages.length; i++) {
+      this.supportedLanguageMap_[supportedLanguages[i].code] =
+          supportedLanguages[i];
+    }
+
+    // Create a list of enabled language info from the supported languages.
+    var enabledLanguages = this.getEnabledLanguages_();
+    // Populate the hash map of enabled languages.
+    for (var i = 0; i < enabledLanguages.length; i++) {
+      var languageInfo = enabledLanguages[i];
+      this.enabledLanguageMap_[languageInfo.language.code] = languageInfo;
+    }
+
+    // Initialize the Polymer model.
+    this.model = {
+      supportedLanguages: supportedLanguages,
+      enabledLanguages: enabledLanguages,
+    };
+  },
+
+  /**
+   * Returns a list of LanguageInfos for each enabled language in the supported
+   * languages list.
+   * @private
+   * @return {!Array<!LanguageInfo>}
+   */
+  getEnabledLanguages_: function() {
+    var languageCodes = this.get(preferredLanguagesPath).split(',');

[stevenjb, 2015/09/22 01:45:15]

Could this.preferredLanguagesPath be undefined if the pref isn't set for some
reason? It seems like something we probably shouldn't assume here and should
handle (maybe log an error) instead of calling 'split' on undefined.

[michaelpg, 2015/09/22 04:03:15]

Once prefs are initialized, I believe every pref should always be set, and all
of our code would be a lot simpler with that assumption. Whether they have a
user value or just a default value, the value won't be undefined.

If that assumption is wrong or we break it (e.g. by changing pref names),
functionality would be broken anyway, and the errors here will alert us to that.

TL;DR: I don't want to add a lot of guards for conditions that shouldn't happen.
I'll add some asserts to ensure these functions are only called after prefs are
initialized.

[stevenjb, 2015/09/22 16:47:33]

Hmm. I'm not super excited about a typo in C++ code (or removal of a pref)
causing, e.g. 'languageCodes.length' to fail as an undefined reference. I don't
feel that the reason for failure will be abundantly clear.

I'd love to figure out a way to add an assert to these getters. Maybe attach a
method to |prefs|? Or a CrSettingsPrefs method? Maybe we can discuss this in
today's meeting.

+    var enabledLanguages = [];
+    var spellCheckMap = this.getSpellCheckMap_();
+    for (var i = 0; i < languageCodes.length; i++) {
+      var code = languageCodes[i];
+      var language = this.supportedLanguageMap_[code];
+      if (!language)
+        continue;
+      var state = {spellCheckEnabled: !!spellCheckMap[code]};
+      enabledLanguages.push({language: language, state: state});
+    }
+    return enabledLanguages;
+  },
+
+  /**
+   * Creates a map whose keys are languages enabled for spell check.
+   * @return {!Object<boolean>}
+   */
+  getSpellCheckMap_: function() {
+    var spellCheckPref = /** @type {chrome.settingsPrivate.PrefObject} */(
+        this.get('prefs.spellcheck.dictionaries'));

[stevenjb, 2015/09/22 01:45:16]

Here too, maybe handle spellCheckPref == undefined

+    var spellCheckCodes = spellCheckPref.value;
+    var spellCheckMap = {};
+    for (var i = 0; i < spellCheckCodes.length; i++)
+      spellCheckMap[spellCheckCodes[i]] = true;
+    return spellCheckMap;
+  },
+
+  /** @private */
+  preferredLanguagesPrefChanged_: function() {
+    if (!this.initialized_)
+      return;
+
+    var enabledLanguages = this.getEnabledLanguages_();
+    // Reset the enabled language map. Do this before notifying of the change
+    // via model.enabledLanguages.
+    this.enabledLanguageMap_ = {};
+    for (var i = 0; i < enabledLanguages.length; i++) {
+      var languageInfo = enabledLanguages[i];
+      this.enabledLanguageMap_[languageInfo.language.code] = languageInfo;
+    }
+    this.set('model.enabledLanguages', enabledLanguages);
+  },
+
+  /**
+   * Updates the spellCheckEnabled state of each enabled language.
+   * @private
+   */
+  spellCheckDictionariesPrefChanged_: function() {
+    if (!this.initialized_)
+      return;
+
+    var spellCheckMap = this.getSpellCheckMap_();
+    for (var i = 0; i < this.model.enabledLanguages.length; i++) {
+      this.set('model.enabledLanguages.' + i + '.state.spellCheckEnabled',
+               !!spellCheckMap[this.model.enabledLanguages[i].language.code]);
+    }
+  },
+
+  /**
+   * On supported systems, sets the prospective UI language to the chosen
+   * language. This dosen't affect the actual UI language until a restart.
+   * @param {string} languageCode
+   */
+  setUILanguage: function(languageCode) {
+    // Set the prospective UI language. This won't take effect until a restart.
+    if (cr.isWindows)
+      this.set('prefs.intl.app_locale.value', languageCode);
+    else if (cr.isChromeOS)
+      chrome.send('setUILanguage', [languageCode]);

[stevenjb, 2015/09/22 01:45:15]

FWIW: Since we're using a chrome.send call here anyway, I would personally
rather do the win/cros logic there.

[michaelpg, 2015/09/22 04:03:15]

Good call -- we should be setting the LocalState pref on Windows anyway.

+  },
+
+  /**
+   * Enables the language, making it available for spell check and input.
+   * @param {string} languageCode
+   */
+  enableLanguage: function(languageCode) {
+    var languageCodes = this.get(preferredLanguagesPath);

[stevenjb, 2015/09/22 01:45:16]

Handle undefined?

+    var index = languageCodes.split(',').indexOf(languageCode);
+    if (index > -1)
+      return;
+    this.set(preferredLanguagesPath, languageCodes + ',' + languageCode);
+  },
+
+  /**
+   * Disables the language.
+   * @param {string} languageCode
+   */
+  disableLanguage: function(languageCode) {
+    // Don't disable the UI language.
+    var appLocale = this.get('prefs.intl.app_locale.value') ||
+                    navigator.language;

[stevenjb, 2015/09/22 01:45:15]

Handle undefined?

+    if (languageCode == appLocale)
+      return;
+
+    var languageCodes = this.get(preferredLanguagesPath).split(',');

[stevenjb, 2015/09/22 01:45:15]

Handle undefined?

+    // Don't disable the only enabled language.
+    if (languageCodes.length == 1)
+      return;
+
+    // Remove the language from spell check.
+    var spellCheckIndex =
+        this.get('prefs.spellcheck.dictionaries.value').indexOf(languageCode);
+    if (spellCheckIndex != -1)
+      this.splice('prefs.spellcheck.dictionaries.value', spellCheckIndex, 1);

[stevenjb, 2015/09/22 01:45:15]

Handle undefined?

+
+    var languageIndex = languageCodes.indexOf(languageCode);
+    if (languageIndex == -1)
+      return;
+    languageCodes.splice(languageIndex, 1);
+    this.set(preferredLanguagesPath, languageCodes.join(','));
+  },
+
+  /**
+   * @param {string} languageCode
+   * @return {boolean} True if the language is enabled.
+   */
+  isEnabled: function(languageCode) {
+    return !!this.enabledLanguageMap_[languageCode];
+  },
+
+  /**
+   * Enables or disables spell check for the given language.
+   * @param {string} languageCode
+   * @param {boolean} enable
+   */
+  toggleSpellCheck: function(languageCode, enable) {
+    var spellCheckPref = /** @type {chrome.settingsPrivate.PrefObject} */(
+        this.get('prefs.spellcheck.dictionaries'));

[stevenjb, 2015/09/22 01:45:15]

Handle undefined?

+    if (enable) {
+      if (spellCheckPref.value.indexOf(languageCode) == -1)
+        this.push('prefs.spellcheck.dictionaries.value', languageCode);
+    } else {
+      // TODO: need update externs before committing
+      this.arrayDelete('prefs.spellcheck.dictionaries.value', languageCode);
+    }
+  },
+});
+})();