From 6e75443936b14f74d96af2c94af110a4957d693e Mon Sep 17 00:00:00 2001 From: Brian Grinstead Date: Mon, 7 Jul 2014 13:52:00 +0200 Subject: [PATCH] Bug 1031472 - Automatically reload all preferences in source editor. r=vporof,harth --- browser/devtools/markupview/html-editor.js | 3 +- .../devtools/netmonitor/netmonitor-view.js | 9 ++- browser/devtools/projecteditor/lib/editors.js | 5 -- browser/devtools/scratchpad/scratchpad.js | 7 +- browser/devtools/shadereditor/shadereditor.js | 16 +++- browser/devtools/sourceeditor/autocomplete.js | 41 ++++++---- browser/devtools/sourceeditor/editor.js | 68 +++++++++++++--- .../devtools/sourceeditor/test/browser.ini | 1 + .../test/browser_editor_autocomplete_basic.js | 2 - .../sourceeditor/test/browser_editor_prefs.js | 78 +++++++++++++++++++ .../devtools/styleeditor/StyleSheetEditor.jsm | 24 +++--- 11 files changed, 201 insertions(+), 53 deletions(-) create mode 100644 browser/devtools/sourceeditor/test/browser_editor_prefs.js diff --git a/browser/devtools/markupview/html-editor.js b/browser/devtools/markupview/html-editor.js index 6f3b7609fab4..244e854f64a9 100644 --- a/browser/devtools/markupview/html-editor.js +++ b/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(); } }; diff --git a/browser/devtools/netmonitor/netmonitor-view.js b/browser/devtools/netmonitor/netmonitor-view.js index cd2173ac5931..dcd4e6e8cace 100644 --- a/browser/devtools/netmonitor/netmonitor-view.js +++ b/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. diff --git a/browser/devtools/projecteditor/lib/editors.js b/browser/devtools/projecteditor/lib/editors.js index 2cfaa695a9c7..98a303830c36 100644 --- a/browser/devtools/projecteditor/lib/editors.js +++ b/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(); - } - }); }, /** diff --git a/browser/devtools/scratchpad/scratchpad.js b/browser/devtools/scratchpad/scratchpad.js index 688de64982e4..3fd7654ecc69 100644 --- a/browser/devtools/scratchpad/scratchpad.js +++ b/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); }, diff --git a/browser/devtools/shadereditor/shadereditor.js b/browser/devtools/shadereditor/shadereditor.js index 359c80429b4d..7febb0e617ce 100644 --- a/browser/devtools/shadereditor/shadereditor.js +++ b/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; }, diff --git a/browser/devtools/sourceeditor/autocomplete.js b/browser/devtools/sourceeditor/autocomplete.js index f9d330c3ee19..1fcab2b75e59 100644 --- a/browser/devtools/sourceeditor/autocomplete.js +++ b/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; diff --git a/browser/devtools/sourceeditor/editor.js b/browser/devtools/sourceeditor/editor.js index 6bc200b9e5ca..019b17e15326 100644 --- a/browser/devtools/sourceeditor/editor.js +++ b/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"); } }; diff --git a/browser/devtools/sourceeditor/test/browser.ini b/browser/devtools/sourceeditor/test/browser.ini index d5f6040555d8..6c6bf0a7cf0f 100644 --- a/browser/devtools/sourceeditor/test/browser.ini +++ b/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] diff --git a/browser/devtools/sourceeditor/test/browser_editor_autocomplete_basic.js b/browser/devtools/sourceeditor/test/browser_editor_autocomplete_basic.js index 5d211ea7bee5..025014e43a40 100644 --- a/browser/devtools/sourceeditor/test/browser_editor_autocomplete_basic.js +++ b/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"); diff --git a/browser/devtools/sourceeditor/test/browser_editor_prefs.js b/browser/devtools/sourceeditor/test/browser_editor_prefs.js new file mode 100644 index 000000000000..04e55b0e6eb1 --- /dev/null +++ b/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); + }); +} diff --git a/browser/devtools/styleeditor/StyleSheetEditor.jsm b/browser/devtools/styleeditor/StyleSheetEditor.jsm index 8ed581a02559..afee0a20abc6 100644 --- a/browser/devtools/styleeditor/StyleSheetEditor.jsm +++ b/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);