Bug 1535955 - fix display of current selection of charset-picker menulists. r=khushil,jorgk a=jorgk
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Sat, 30 Mar 2019 21:52:04 +0100
changeset 33972 d9af5609d45e
parent 33971 1599c84ad552
child 33973 6812e206e6d8
push id2394
push usermozilla@jorgk.com
push dateSat, 30 Mar 2019 21:10:54 +0000
treeherdercomm-beta@a07bc5354c40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhushil, jorgk, jorgk
bugs1535955
Bug 1535955 - fix display of current selection of charset-picker menulists. r=khushil,jorgk a=jorgk Change the pickers to be one class per "type" instead of using the subset attribute. Having to rebuild the menus to change the subset causes problems with the internal state keeping in MozMenuList (since MozMenuList.removeAll() will cause this.selectedItem = null). Also important to pass on the arguments for attributeChangedCallback to super so it can do the right thing!
mail/components/preferences/fonts.xul
mailnews/base/content/folderProps.xul
mailnews/base/content/menulist-charsetpicker.js
mailnews/base/prefs/content/am-server.xul
--- a/mail/components/preferences/fonts.xul
+++ b/mail/components/preferences/fonts.xul
@@ -278,27 +278,25 @@
           <column flex="1"/>
         </columns>
 
         <rows>
           <row align="center" class="indent">
             <label control="sendDefaultCharsetList"
                    value="&sendDefaultCharset.label;"
                    accesskey="&sendDefaultCharset.accesskey;"/>
-            <menulist is="menulist-charsetpicker" id="sendDefaultCharsetList"
-                      subset="sending"
+            <menulist is="menulist-charsetpicker-sending" id="sendDefaultCharsetList"
                       preference="mailnews.send_default_charset"/>
           </row>
 
           <row align="center" class="indent">
             <label control="viewDefaultCharsetList"
                    value="&viewDefaultCharsetList.label;"
                    accesskey="&viewDefaultCharsetList.accesskey;"/>
-            <menulist is="menulist-charsetpicker" id="viewDefaultCharsetList"
-                      subset="viewing"
+            <menulist is="menulist-charsetpicker-viewing" id="viewDefaultCharsetList"
                       preference="mailnews.view_default_charset"/>
           </row>
         </rows>
       </grid>
 
       <separator class="thin"/>
 
       <checkbox id="replyInDefaultCharset" label="&replyInDefaultCharset3.label;"
--- a/mailnews/base/content/folderProps.xul
+++ b/mailnews/base/content/folderProps.xul
@@ -62,17 +62,17 @@
                 id="folderCheckForNewMessages"
                 label="&folderCheckForNewMessages2.label;"
                 accesskey="&folderCheckForNewMessages2.accesskey;"/>
       <vbox>
         <hbox align="center" valign="middle">
           <label value="&folderCharsetFallback2.label;"
                  accesskey="&folderCharsetFallback2.accesskey;"
                  control="folderCharsetList"/>
-          <menulist is="menulist-charsetpicker" id="folderCharsetList" subset="viewing"/>
+          <menulist is="menulist-charsetpicker-viewing" id="folderCharsetList"/>
         </hbox>
         <checkbox class="indent" id="folderCharsetOverride"
                   label="&folderCharsetEnforce2.label;"
                   accesskey="&folderCharsetEnforce2.accesskey;"/>
       </vbox>
       <groupbox id="folderRebuildSummaryGroupBox" align="baseline">
         <grid>
           <columns>
--- a/mailnews/base/content/menulist-charsetpicker.js
+++ b/mailnews/base/content/menulist-charsetpicker.js
@@ -3,102 +3,118 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 customElements.whenDefined("menulist").then(() => {
   const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
   /**
    * MozMenulistCharsetpicker is a menulist widget that is automatically
    * populated with charset selections.
-   * - Setting subset="sending" will show the values applicable for sending
-   * - Setting subset="viewing" will show the values applicable for viewing
-   * - Setting preference="<name>" will set the selected value to that of the named preference value
+   * Setting preference="<name>" will set the selected value to that of the
+   * named preference value.
+   * @abstract
    * @extends {MozMenuList}
    */
-  class MozMenulistCharsetpicker extends customElements.get("menulist") {
+  class MozMenulistCharsetpickerBase extends customElements.get("menulist") {
     static get observedAttributes() {
-      return ["subset", "preference"];
+      return super.observedAttributes.concat(["subset", "preference"]);
+    }
+
+    /**
+     * Get the charset values to show in the list.
+     * @abstract
+     * @return {String[]} an array of character encoding names
+     */
+    get charsetValues() {
+      return [];
     }
 
     connectedCallback() {
       super.connectedCallback();
       if (this.delayConnectedCallback()) {
         return;
       }
 
       if (this.menupopup) {
         return;
       }
 
-      this._setupCharsets();
-      this._setupSelectedValueFromPref();
-    }
-
-    _setupCharsets() {
-      let charsetValues;
-      if (this.getAttribute("subset") == "sending") {
-        charsetValues = [
-          "UTF-8", "EUC-KR", "gbk", "gb18030", "ISO-2022-JP",
-          "ISO-8859-1", "ISO-8859-7", "windows-1252",
-        ];
-      } else if (this.getAttribute("subset") == "viewing") {
-        charsetValues = [
-          "UTF-8", "Big5", "EUC-KR", "gbk", "ISO-2022-JP",
-          "ISO-8859-1", "ISO-8859-2", "ISO-8859-7",
-          "windows-874", "windows-1250", "windows-1251",
-          "windows-1252", "windows-1255", "windows-1256",
-          "windows-1257", "windows-1258",
-        ];
-      }
-
       let charsetBundle = Services.strings.createBundle(
         "chrome://messenger/locale/charsetTitles.properties");
-      let menuLabels = charsetValues.map((item) => {
+      this.charsetValues.map((item) => {
         let strCharset = charsetBundle.GetStringFromName(
           item.toLowerCase() + ".title");
         return { label: strCharset, value: item };
-      });
-
-      menuLabels.sort((a, b) => {
+      }).sort((a, b) => {
         if (a.value == "UTF-8" || a.label < b.label) {
           return -1;
         } else if (b.value == "UTF-8" || a.label > b.label) {
           return 1;
         }
         return 0;
-      });
-
-      menuLabels.forEach((item) => {
+      }).forEach((item) => {
         this.appendItem(item.label, item.value);
       });
+      this._setupSelectedValueFromPref();
     }
 
     _setupSelectedValueFromPref() {
       // Set appropriate selected menu item based on preference value.
       if (this.hasAttribute("preference")) {
         let preference = Services.prefs.getComplexValue(
           this.getAttribute("preference"), Ci.nsIPrefLocalizedString);
         this.value = preference.data;
       }
     }
 
-    attributeChangedCallback() {
-      super.attributeChangedCallback();
-      if (!this.isConnectedAndReady) {
+    attributeChangedCallback(name, oldValue, newValue) {
+      super.attributeChangedCallback(name, oldValue, newValue);
+      // @see MozElementMixin.attributeChangedCallback()
+      if (!this.isConnectedAndReady || oldValue === newValue || !this.inheritedAttributesCache) {
         return;
       }
-      this._updateAttributes();
+      if (name == "preference") {
+        this._setupSelectedValueFromPref();
+      }
     }
+  }
 
-    _updateAttributes() {
-      this.removeAllItems();
-      this._setupCharsets();
-      this._setupSelectedValueFromPref();
+  /**
+   * Menulist widget that shows charset applicable for sending messages.
+   * @extends MozMenulistCharsetpickerBase
+   */
+  class MozMenulistCharsetpickerSending extends MozMenulistCharsetpickerBase {
+    get charsetValues() {
+      return [
+        "UTF-8", "EUC-KR", "gbk", "gb18030", "ISO-2022-JP",
+        "ISO-8859-1", "ISO-8859-7", "windows-1252",
+      ];
     }
   }
-  customElements.define("menulist-charsetpicker", MozMenulistCharsetpicker, { extends: "menulist" });
+  customElements.define("menulist-charsetpicker-sending",
+    MozMenulistCharsetpickerSending, { extends: "menulist" }
+  );
+
+  /**
+   * Menulist widget that shows charsets applicable for viewing messages.
+   * @extends MozMenulistCharsetpickerBase
+   */
+  class MozMenulistCharsetpickerViewing extends MozMenulistCharsetpickerBase {
+    get charsetValues() {
+      return [
+        "UTF-8", "Big5", "EUC-KR", "gbk", "ISO-2022-JP",
+        "ISO-8859-1", "ISO-8859-2", "ISO-8859-7",
+        "windows-874", "windows-1250", "windows-1251",
+        "windows-1252", "windows-1255", "windows-1256",
+        "windows-1257", "windows-1258",
+      ];
+    }
+  }
+  customElements.define("menulist-charsetpicker-viewing",
+    MozMenulistCharsetpickerViewing, { extends: "menulist" }
+  );
 });
 
 // The menulist CE is defined lazily. Create one now to get menulist defined,
 // allowing us to inherit from it.
 if (!customElements.get("menulist")) {
   delete document.createElement("menulist");
 }
--- a/mailnews/base/prefs/content/am-server.xul
+++ b/mailnews/base/prefs/content/am-server.xul
@@ -451,17 +451,17 @@
       </vbox>
 
     </groupbox>
 
     <separator class="thin"/>
 
     <hbox hidefor="imap,pop3,movemail" align="center" valign="middle" iscontrolcontainer="true">
       <label value="&serverDefaultCharset2.label;" control="nntp.charset"/>
-      <menulist is="menulist-charsetpicker" id="nntp.charset" subset="viewing"
+      <menulist is="menulist-charsetpicker-viewing" id="nntp.charset"
                 hidable="true"
                 hidefor="imap,pop3,movemail"
                 wsm_persist="true"
                 preftype="string"
                 prefstring="mail.server.%serverkey%.charset"/>
     </hbox>
   </vbox>
 </page>