Bug 1031472 - Automatically reload all preferences in source editor. r=vporof,harth

browser/devtools/markupview/html-editor.js

@@ -181,6 +181,7 @@ HTMLEditor.prototype = {
     this.editorInner.removeEventListener("click", stopPropagation, false);
 
     this.hide(false);
-    this.container.parentNode.removeChild(this.container);
+    this.container.remove();
+    this.editor.destroy();
   }
 };

browser/devtools/netmonitor/netmonitor-view.js

@@ -118,7 +118,7 @@ let NetMonitorView = {
   /**
    * Destroys the UI for all the displayed panes.
    */
-  _destroyPanes: function() {
+  _destroyPanes: Task.async(function*() {
     dumpn("Destroying the NetMonitorView panes");
 
     Prefs.networkDetailsWidth = this._detailsPane.getAttribute("width");
@@ -126,7 +126,12 @@ let NetMonitorView = {
 
     this._detailsPane = null;
     this._detailsPaneToggleButton = null;
-  },
+
+    for (let p of this._editorPromises.values()) {
+      let editor = yield p;
+      editor.destroy();
+    }
+  }),
 
   /**
    * Gets the visibility state of the network details pane.

browser/devtools/projecteditor/lib/editors.js

@@ -165,11 +165,6 @@ var TextEditor = Class({
     });
 
     this.appended = this.editor.appendTo(this.elt);
-    this.appended.then(() => {
-      if (this.editor) {
-        this.editor.setupAutoCompletion();
-      }
-    });
   },
 
   /**

browser/devtools/scratchpad/scratchpad.js

@@ -1613,7 +1613,6 @@ var Scratchpad = {
     this.editor.appendTo(editorElement).then(() => {
       var lines = initialText.split("\n");
 
-      this.editor.setupAutoCompletion();
       this.editor.on("change", this._onChanged);
       this._onPaste = WebConsoleUtils.pasteHandlerGen(this.editor.container.contentDocument.body,
                                                       document.querySelector('#scratchpad-notificationbox'));
@@ -2323,6 +2322,12 @@ var CloseObserver = {
 
   uninit: function CO_uninit()
   {
+    // Will throw exception if removeObserver is called twice.
+    if (this._uninited) {
+      return;
+    }
+
+    this._uninited = true;
     Services.obs.removeObserver(this, "browser-lastwindow-close-requested",
                                 false);
   },

browser/devtools/shadereditor/shadereditor.js

@@ -359,9 +359,14 @@ let ShadersEditorsView = {
   /**
    * Destruction function, called when the tool is closed.
    */
-  destroy: function() {
+  destroy: Task.async(function*() {
+    this._destroyed = true;
     this._toggleListeners("off");
-  },
+    for (let p of this._editorPromises.values()) {
+      let editor = yield p;
+      editor.destroy();
+    }
+  }),
 
   /**
    * Sets the text displayed in the vertex and fragment shader editors.
@@ -415,7 +420,12 @@ let ShadersEditorsView = {
     let parent = $("#" + type +"-editor");
     let editor = new Editor(DEFAULT_EDITOR_CONFIG);
     editor.config.mode = Editor.modes[type];
-    editor.appendTo(parent).then(() => deferred.resolve(editor));
+
+    if (this._destroyed) {
+      deferred.resolve(editor);
+    } else {
+      editor.appendTo(parent).then(() => deferred.resolve(editor));
+    }
 
     return deferred.promise;
   },

browser/devtools/sourceeditor/autocomplete.js

@@ -11,14 +11,14 @@ const CM_TERN_SCRIPTS = [
   "chrome://browser/content/devtools/codemirror/show-hint.js"
 ];
 
-const privates = new WeakMap();
+const autocompleteMap = new WeakMap();
 
 /**
  * Prepares an editor instance for autocompletion.
  */
 function initializeAutoCompletion(ctx, options = {}) {
   let { cm, ed, Editor } = ctx;
-  if (privates.has(ed)) {
+  if (autocompleteMap.has(ed)) {
     return;
   }
 
@@ -88,12 +88,12 @@ function initializeAutoCompletion(ctx, options = {}) {
       cm.off("cursorActivity", updateArgHintsCallback);
       cm.removeKeyMap(keyMap);
       win.tern = cm.tern = null;
-      privates.delete(ed);
+      autocompleteMap.delete(ed);
     };
 
     ed.on("destroy", destroyTern);
 
-    privates.set(ed, {
+    autocompleteMap.set(ed, {
       destroy: destroyTern
     });
 
@@ -126,8 +126,8 @@ function initializeAutoCompletion(ctx, options = {}) {
     "Up": cycle.bind(null, true),
     "Enter": () => {
       if (popup && popup.isOpen) {
-        if (!privates.get(ed).suggestionInsertedOnce) {
-          privates.get(ed).insertingSuggestion = true;
+        if (!autocompleteMap.get(ed).suggestionInsertedOnce) {
+          autocompleteMap.get(ed).insertingSuggestion = true;
           let {label, preLabel, text} = popup.getItemAtIndex(0);
           let cur = ed.getCursor();
           ed.replaceText(text.slice(preLabel.length), cur, cur);
@@ -157,10 +157,10 @@ function initializeAutoCompletion(ctx, options = {}) {
     cm.removeKeyMap(keyMap);
     popup.destroy();
     keyMap = popup = completer = null;
-    privates.delete(ed);
+    autocompleteMap.delete(ed);
   }
 
-  privates.set(ed, {
+  autocompleteMap.set(ed, {
     popup: popup,
     completer: completer,
     keyMap: keyMap,
@@ -175,11 +175,11 @@ function initializeAutoCompletion(ctx, options = {}) {
  */
 function destroyAutoCompletion(ctx) {
   let { ed } = ctx;
-  if (!privates.has(ed)) {
+  if (!autocompleteMap.has(ed)) {
     return;
   }
 
-  let {destroy} = privates.get(ed);
+  let {destroy} = autocompleteMap.get(ed);
   destroy();
 }
 
@@ -187,7 +187,7 @@ function destroyAutoCompletion(ctx) {
  * Provides suggestions to autocomplete the current token/word being typed.
  */
 function autoComplete({ ed, cm }) {
-  let private = privates.get(ed);
+  let private = autocompleteMap.get(ed);
   let { completer, popup } = private;
   if (!completer || private.insertingSuggestion || private.doNotAutocomplete) {
     private.insertingSuggestion = false;
@@ -223,7 +223,7 @@ function autoComplete({ ed, cm }) {
  * when `reverse` is not true. Opposite otherwise.
  */
 function cycleSuggestions(ed, reverse) {
-  let private = privates.get(ed);
+  let private = autocompleteMap.get(ed);
   let { popup, completer } = private;
   let cur = ed.getCursor();
   private.insertingSuggestion = true;
@@ -263,7 +263,7 @@ function cycleSuggestions(ed, reverse) {
  * keypresses.
  */
 function onEditorKeypress({ ed, Editor }, cm, event) {
-  let private = privates.get(ed);
+  let private = autocompleteMap.get(ed);
 
   // Do not try to autocomplete with multiple selections.
   if (ed.hasMultipleSelections()) {
@@ -319,8 +319,8 @@ function onEditorKeypress({ ed, Editor }, cm, event) {
  * Returns the private popup. This method is used by tests to test the feature.
  */
 function getPopup({ ed }) {
-  if (privates.has(ed))
-    return privates.get(ed).popup;
+  if (autocompleteMap.has(ed))
+    return autocompleteMap.get(ed).popup;
 
   return null;
 }
@@ -330,16 +330,25 @@ function getPopup({ ed }) {
  * implementation of completer supports it.
  */
 function getInfoAt({ ed }, caret) {
-  let completer = privates.get(ed).completer;
+  let completer = autocompleteMap.get(ed).completer;
   if (completer && completer.getInfoAt)
     return completer.getInfoAt(ed.getText(), caret);
 
   return null;
 }
 
+/**
+ * Returns whether autocompletion is enabled for this editor.
+ * Used for testing
+ */
+function isAutocompletionEnabled({ ed }) {
+  return autocompleteMap.has(ed);
+}
+
 // Export functions
 
 module.exports.initializeAutoCompletion = initializeAutoCompletion;
 module.exports.destroyAutoCompletion = destroyAutoCompletion;
 module.exports.getAutocompletionPopup = getPopup;
 module.exports.getInfoAt = getInfoAt;
+module.exports.isAutocompletionEnabled = isAutocompletionEnabled;

browser/devtools/sourceeditor/editor.js

@@ -17,6 +17,7 @@ const DETECT_INDENT = "devtools.editor.detectindentation";
 const DETECT_INDENT_MAX_LINES = 500;
 const L10N_BUNDLE = "chrome://browser/locale/devtools/sourceeditor.properties";
 const XUL_NS      = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
+const VALID_KEYMAPS = new Set(["emacs", "vim", "sublime"]);
 
 // Maximum allowed margin (in number of lines) from top or bottom of the editor
 // while shifting to a line which was initially out of view.
@@ -29,6 +30,7 @@ const RE_JUMP_TO_LINE = /^(\d+):?(\d+)?/;
 
 const {Promise: promise} = Cu.import("resource://gre/modules/Promise.jsm", {});
 const events  = require("devtools/toolkit/event-emitter");
+const { PrefObserver } = require("devtools/styleeditor/utils");
 
 Cu.import("resource://gre/modules/Services.jsm");
 const L10N = Services.strings.createBundle(L10N_BUNDLE);
@@ -139,7 +141,6 @@ Editor.modes = {
 function Editor(config) {
   const tabSize = Services.prefs.getIntPref(TAB_SIZE);
   const useTabs = !Services.prefs.getBoolPref(EXPAND_TAB);
-  const keyMap = Services.prefs.getCharPref(KEYMAP);
   const useAutoClose = Services.prefs.getBoolPref(AUTO_CLOSE);
 
   this.version = null;
@@ -157,7 +158,8 @@ function Editor(config) {
     autoCloseEnabled:  useAutoClose,
     theme:             "mozilla",
     themeSwitching:    true,
-    autocomplete:      false
+    autocomplete:      false,
+    autocompleteOpts:  {}
   };
 
   // Additional shortcuts.
@@ -170,9 +172,6 @@ function Editor(config) {
   this.config.extraKeys[Editor.keyFor("indentLess")] = false;
   this.config.extraKeys[Editor.keyFor("indentMore")] = false;
 
-  // If alternative keymap is provided, use it.
-  if (keyMap === "emacs" || keyMap === "vim" || keyMap === "sublime")
-    this.config.keyMap = keyMap;
 
   // Overwrite default config with user-provided, if needed.
   Object.keys(config).forEach((k) => {
@@ -199,9 +198,8 @@ function Editor(config) {
     }
   }
 
-  // Configure automatic bracket closing.
-  if (!this.config.autoCloseEnabled)
-    this.config.autoCloseBrackets = false;
+  // Remember the initial value of autoCloseBrackets.
+  this.config.autoCloseBracketsSaved = this.config.autoCloseBrackets;
 
   // Overwrite default tab behavior. If something is selected,
   // indent those lines. If nothing is selected and we're
@@ -325,8 +323,16 @@ Editor.prototype = {
       this.container = env;
       editors.set(this, cm);
 
-      this.resetIndentUnit();
+      this.reloadPreferences = this.reloadPreferences.bind(this);
+      this._prefObserver = new PrefObserver("devtools.editor.");
+      this._prefObserver.on(TAB_SIZE, this.reloadPreferences);
+      this._prefObserver.on(EXPAND_TAB, this.reloadPreferences);
+      this._prefObserver.on(KEYMAP, this.reloadPreferences);
+      this._prefObserver.on(AUTO_CLOSE, this.reloadPreferences);
+      this._prefObserver.on(AUTOCOMPLETE, this.reloadPreferences);
+      this._prefObserver.on(DETECT_INDENT, this.reloadPreferences);
 
+      this.reloadPreferences();
       def.resolve();
     };
 
@@ -397,6 +403,28 @@ Editor.prototype = {
     this.resetIndentUnit();
   },
 
+  /**
+   * Reload the state of the editor based on all current preferences.
+   * This is called automatically when any of the relevant preferences
+   * change.
+   */
+  reloadPreferences: function() {
+    // Restore the saved autoCloseBrackets value if it is preffed on.
+    let useAutoClose = Services.prefs.getBoolPref(AUTO_CLOSE);
+    this.setOption("autoCloseBrackets",
+      useAutoClose ? this.config.autoCloseBracketsSaved : false);
+
+    // If alternative keymap is provided, use it.
+    const keyMap = Services.prefs.getCharPref(KEYMAP);
+    if (VALID_KEYMAPS.has(keyMap))
+      this.setOption("keyMap", keyMap)
+    else
+      this.setOption("keyMap", "default");
+
+    this.resetIndentUnit();
+    this.setupAutoCompletion();
+  },
+
   /**
    * Set the editor's indentation based on the current prefs and
    * re-detect indentation if we should.
@@ -878,6 +906,13 @@ Editor.prototype = {
    */
   setOption: function(o, v) {
     let cm = editors.get(this);
+
+    // Save the state of a valid autoCloseBrackets string, so we can reset
+    // it if it gets preffed off and back on.
+    if (o === "autoCloseBrackets" && v) {
+      this.config.autoCloseBracketsSaved = v;
+    }
+
     if (o === "autocomplete") {
       this.config.autocomplete = v;
       this.setupAutoCompletion();
@@ -908,7 +943,7 @@ Editor.prototype = {
    * it just because it is preffed on (it still needs to be requested by the
    * editor), but we do want to always disable it if it is preffed off.
    */
-  setupAutoCompletion: function (options = {}) {
+  setupAutoCompletion: function () {
     // The autocomplete module will overwrite this.initializeAutoCompletion
     // with a mode specific autocompletion handler.
     if (!this.initializeAutoCompletion) {
@@ -916,7 +951,7 @@ Editor.prototype = {
     }
 
     if (this.config.autocomplete && Services.prefs.getBoolPref(AUTOCOMPLETE)) {
-      this.initializeAutoCompletion(options);
+      this.initializeAutoCompletion(this.config.autocompleteOpts);
     } else {
       this.destroyAutoCompletion();
     }
@@ -957,6 +992,17 @@ Editor.prototype = {
     this.container = null;
     this.config = null;
     this.version = null;
+
+    if (this._prefObserver) {
+      this._prefObserver.off(TAB_SIZE, this.reloadPreferences);
+      this._prefObserver.off(EXPAND_TAB, this.reloadPreferences);
+      this._prefObserver.off(KEYMAP, this.reloadPreferences);
+      this._prefObserver.off(AUTO_CLOSE, this.reloadPreferences);
+      this._prefObserver.off(AUTOCOMPLETE, this.reloadPreferences);
+      this._prefObserver.off(DETECT_INDENT, this.reloadPreferences);
+      this._prefObserver.destroy();
+    }
+
     this.emit("destroy");
   }
 };

browser/devtools/sourceeditor/test/browser.ini

@@ -28,6 +28,7 @@ support-files =
 [browser_editor_history.js]
 [browser_editor_markers.js]
 [browser_editor_movelines.js]
+[browser_editor_prefs.js]
 [browser_editor_addons.js]
 [browser_codemirror.js]
 [browser_css_autocompletion.js]

browser/devtools/sourceeditor/test/browser_editor_autocomplete_basic.js

@@ -53,8 +53,6 @@ function testPref(ed, win) {
   info ("Preffing autocompletion off");
   Services.prefs.setBoolPref(AUTOCOMPLETION_PREF, false);
 
-  ed.setupAutoCompletion();
-
   ok (ed.getOption("autocomplete"), "Autocompletion is still set");
   ok (!win.tern, "Tern is no longer defined on the window");
 

browser/devtools/sourceeditor/test/browser_editor_prefs.js

@@ -0,0 +1,78 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test to make sure that the editor reacts to preference changes
+
+const TAB_SIZE    = "devtools.editor.tabsize";
+const EXPAND_TAB  = "devtools.editor.expandtab";
+const KEYMAP      = "devtools.editor.keymap";
+const AUTO_CLOSE  = "devtools.editor.autoclosebrackets";
+const AUTOCOMPLETE  = "devtools.editor.autocomplete";
+const DETECT_INDENT = "devtools.editor.detectindentation";
+
+function test() {
+  waitForExplicitFinish();
+  setup((ed, win) => {
+
+    ed.setText("Checking preferences.");
+
+    info ("Turning prefs off");
+
+    ed.setOption("autocomplete", true);
+
+    Services.prefs.setIntPref(TAB_SIZE, 2);
+    Services.prefs.setBoolPref(EXPAND_TAB, false);
+    Services.prefs.setCharPref(KEYMAP, "default");
+    Services.prefs.setBoolPref(AUTO_CLOSE, false);
+    Services.prefs.setBoolPref(AUTOCOMPLETE, false);
+    Services.prefs.setBoolPref(DETECT_INDENT, false);
+
+    is(ed.getOption("tabSize"), 2, "tabSize is correct");
+    is(ed.getOption("indentUnit"), 2, "indentUnit is correct");
+    is(ed.getOption("indentWithTabs"), true, "indentWithTabs is correct");
+    is(ed.getOption("keyMap"), "default", "keyMap is correct");
+    is(ed.getOption("autoCloseBrackets"), "", "autoCloseBrackets is correct");
+    is(ed.getOption("autocomplete"), true, "autocomplete is correct");
+    ok(!ed.isAutocompletionEnabled(), "Autocompletion is not enabled");
+
+    info ("Turning prefs on");
+
+    Services.prefs.setIntPref(TAB_SIZE, 4);
+    Services.prefs.setBoolPref(EXPAND_TAB, true);
+    Services.prefs.setCharPref(KEYMAP, "sublime");
+    Services.prefs.setBoolPref(AUTO_CLOSE, true);
+    Services.prefs.setBoolPref(AUTOCOMPLETE, true);
+
+    is(ed.getOption("tabSize"), 4, "tabSize is correct");
+    is(ed.getOption("indentUnit"), 4, "indentUnit is correct");
+    is(ed.getOption("indentWithTabs"), false, "indentWithTabs is correct");
+    is(ed.getOption("keyMap"), "sublime", "keyMap is correct");
+    is(ed.getOption("autoCloseBrackets"), "()[]{}''\"\"", "autoCloseBrackets is correct");
+    is(ed.getOption("autocomplete"), true, "autocomplete is correct");
+    ok(ed.isAutocompletionEnabled(), "Autocompletion is enabled");
+
+    info ("Checking indentation detection");
+
+    Services.prefs.setBoolPref(DETECT_INDENT, true);
+
+    ed.setText("Detecting\n\tTabs");
+    is(ed.getOption("indentWithTabs"), true, "indentWithTabs is correct");
+    is(ed.getOption("indentUnit"), 4, "indentUnit is correct");
+
+    ed.setText("body {\n  color:red;\n  a:b;\n}");
+    is(ed.getOption("indentWithTabs"), false, "indentWithTabs is correct");
+    is(ed.getOption("indentUnit"), 2, "indentUnit is correct");
+
+    Services.prefs.clearUserPref(TAB_SIZE);
+    Services.prefs.clearUserPref(EXPAND_TAB);
+    Services.prefs.clearUserPref(KEYMAP);
+    Services.prefs.clearUserPref(AUTO_CLOSE);
+    Services.prefs.clearUserPref(AUTOCOMPLETE);
+    Services.prefs.clearUserPref(DETECT_INDENT);
+
+    teardown(ed, win);
+  });
+}

browser/devtools/styleeditor/StyleSheetEditor.jsm

@@ -94,6 +94,8 @@ function StyleSheetEditor(styleSheet, win, file, isNew, walker) {
   this._onMediaRulesChanged = this._onMediaRulesChanged.bind(this)
   this.checkLinkedFileForChanges = this.checkLinkedFileForChanges.bind(this);
   this.markLinkedFileBroken = this.markLinkedFileBroken.bind(this);
+  this.saveToFile = this.saveToFile.bind(this);
+  this.updateStyleSheet = this.updateStyleSheet.bind(this);
 
   this._focusOnSourceEditorReady = false;
   this.cssSheet.on("property-change", this._onPropertyChange);
@@ -347,23 +349,18 @@ StyleSheetEditor.prototype = {
       autoCloseBrackets: "{}()[]",
       extraKeys: this._getKeyBindings(),
       contextMenu: "sourceEditorContextMenu",
-      autocomplete: Services.prefs.getBoolPref(AUTOCOMPLETION_PREF)
+      autocomplete: Services.prefs.getBoolPref(AUTOCOMPLETION_PREF),
+      autocompleteOpts: { walker: this.walker }
     };
-    let sourceEditor = new Editor(config);
+    let sourceEditor = this._sourceEditor = new Editor(config);
 
     sourceEditor.on("dirty-change", this._onPropertyChange);
 
     return sourceEditor.appendTo(inputElement).then(() => {
-      sourceEditor.setupAutoCompletion({ walker: this.walker });
-
-      sourceEditor.on("save", () => {
-        this.saveToFile();
-      });
+      sourceEditor.on("save", this.saveToFile);
 
       if (this.styleSheet.update) {
-        sourceEditor.on("change", () => {
-          this.updateStyleSheet();
-        });
+        sourceEditor.on("change", this.updateStyleSheet);
       }
 
       this.sourceEditor = sourceEditor;
@@ -631,8 +628,11 @@ StyleSheetEditor.prototype = {
    * Clean up for this editor.
    */
   destroy: function() {
-    if (this.sourceEditor) {
-      this.sourceEditor.destroy();
+    if (this._sourceEditor) {
+      this._sourceEditor.off("dirty-change", this._onPropertyChange);
+      this._sourceEditor.off("save", this.saveToFile);
+      this._sourceEditor.off("change", this.updateStyleSheet);
+      this._sourceEditor.destroy();
     }
     this.cssSheet.off("property-change", this._onPropertyChange);
     this.cssSheet.off("media-rules-changed", this._onMediaRulesChanged);