issue #1155 - Don't allow displaying icon text with Australis theme

lib/main.js

 /*
  * This Source Code is subject to the terms of the Mozilla Public License
  * version 2.0 (the "License"). You can obtain a copy of the License at
  * http://mozilla.org/MPL/2.0/.
  */
 
 Cu.import("resource://gre/modules/Services.jsm");
+let {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", null);
 
 let {Prefs} = require("prefs");
 let {WindowObserver} = require("windowObserver");
 
+let CustomizableUI = null;

[Wladimir Palant, 2014/07/30 09:38:55]

No point keeping a global reference to CustomizableUI around - a flag like
isAustralis will do as well, and it will be more obvious what this is about.

[saroyanm, 2014/07/30 10:56:23]

Done.

+
 var WindowFeature =
 {
   observer: null,
 
   init: function()
   {
     if (!this.observer)
       this.observer = new WindowObserver(this, "ready");
   },
 
@@ skipping 12 matching lines @@
   },
 
   removeFromWindow: function(window)
   {
     if (window.location.href != this.windowUrl)
       return;
     this._removeFromWindow(window);
   }
 }
 
+/**
+ * Object initializing add-on options, observes add-on manager notifications
+ * about add-on options being opened.
+ * @type nsIObserver
+ */
+let optionsObserver =
+{
+  init: function()
+  {
+    Services.obs.addObserver(this, "addon-options-displayed", true);
+    onShutdown.add(function()
+    {
+      Services.obs.removeObserver(this, "addon-options-displayed");
+    }.bind(this));
+  },
+
+  initOptionsDoc: function(/**Document*/ doc)
+  {
+    if (CustomizableUI)
+      doc.getElementById("abpcustomization-icondisplay").collapsed = true;
+  },
+
+  observe: function(subject, topic, data)
+  {
+    let {addonID} = require("info")
+    if (data != addonID)
+      return;
+
+    this.initOptionsDoc(subject.QueryInterface(Ci.nsIDOMDocument));
+  },
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference])
+};
+optionsObserver.init();
+
 var VerticalPreferencesLayout =
 {
   __proto__: WindowFeature,
   windowUrl: "chrome://adblockplus/content/ui/filters.xul",
 
   _applyToWindow: function(window)
   {
     let content = window.document.getElementById("content");
     let splitter = window.document.getElementById("filtersSplitter");
     if (!content || !splitter)
@@ skipping 117 matching lines @@
   "vertical-preferences-layout": VerticalPreferencesLayout,
   "preferences-one-line-subscriptions": OneLineSubscriptions,
   "preferences-remove-actions-button": RemoveActionsButton,
   "toolbar-icon-display": ToolbarIconDisplay,
   "green-icon": GreenIcon,
   "remove-menus": RemoveMenus
 };
 
 function updateFeature(name)
 {
+  try
+  {
+    ({CustomizableUI}) = Cu.import("resource:///modules/CustomizableUI.jsm", null);
+  } catch (e) {}

[Wladimir Palant, 2014/07/30 09:38:55]

Why run this for each feature? It should be best to have this at the top, where
the variable is declared:

var isAustralis = false;
try
{
  Cu.import("resource:///modules/CustomizableUI.jsm", null);
  isAustralis = true;
} catch(e) {}

[saroyanm, 2014/07/30 10:56:23]

Oops, too inattentive today!

+
   if (name in features)
   {
     let enabled;
     if (name == "addon-page-styles")
       enabled = true;

[Wladimir Palant, 2014/07/30 09:38:55]

Actually, seeing this - it's probably cleaner to implement hiding of the
preference the same way, without any observer. We can add a new
StylesheetFeature similar to AddonPageStyles and have |enabled = isAustralis|
here so that the stylesheet will only be applied if Australis theme is active.

[saroyanm, 2014/07/30 10:56:23]

Awesome, great idea. How could I miss that.

+    else if (name == "toolbar-icon-display" && CustomizableUI)
+      enabled = false;
     else
       enabled = Prefs[name];
 
     if (enabled)
       features[name].init();
     else
       features[name].shutdown();
   }
 }
 
-
 // Initialize features and make sure to update them on changes
 for (let feature in features)
   updateFeature(feature);
 
 Prefs.addListener(updateFeature);