Fix LiveDevelopment initialization.

busykai · busykai · commit eef9c68a1fdf · 2015-01-15T03:08:11.000-03:00
PreferencesManager.on("change") was executing before AppInit.appReady(). This was causing various inconsistencies, including setting up the UI before LiveDevelopment is actually initialized. Handling of pref change is now moved to AppInit.appReady and the implementation is set only after the modules are actually initialized. Fix #10239: toggle menu to switch livedev impls. - Ensure correct status changes when switching implementations. - Disable 'Project Settings' when multi-browser is on. Also: - Minor improvements to how the pref is defined. - Close LiveDevelopment explicitly after each test. - Minor refactor/doc changes in LiveDevelopment/main.js - Minor stylistic and documentation changes.
diff --git a/src/LiveDevelopment/LiveDevMultiBrowser.js b/src/LiveDevelopment/LiveDevMultiBrowser.js
@@ -467,10 +467,13 @@ define(function (require, exports, module) {
     }
 
     /**
-     * Close all active connections
+     * Closes all active connections.
+     * Returns a resolved promise for API compatibility.
+     * @return {$.Promise} A resolved promise
      */
     function close() {
-        return _close(true);
+        _close(true);
+        return new $.Deferred().resolve().promise();
     }
     
     /**
diff --git a/src/LiveDevelopment/LiveDevelopment.js b/src/LiveDevelopment/LiveDevelopment.js
@@ -52,6 +52,7 @@
  *      2: Loading agents
  *      3: Active
  *      4: Out of sync
+ *      5: Sync error
  *
  * The reason codes are:
  * - null (Unknown reason)
diff --git a/src/LiveDevelopment/main.js b/src/LiveDevelopment/main.js
@@ -40,6 +40,7 @@ define(function main(require, exports, module) {
         Commands            = require("command/Commands"),
         AppInit             = require("utils/AppInit"),
         LiveDevelopment     = require("LiveDevelopment/LiveDevelopment"),
+        MultiBrowserLiveDev = require("LiveDevelopment/LiveDevMultiBrowser"),
         Inspector           = require("LiveDevelopment/Inspector/Inspector"),
         CommandManager      = require("command/CommandManager"),
         PreferencesManager  = require("preferences/PreferencesManager"),
@@ -50,9 +51,6 @@ define(function main(require, exports, module) {
         ExtensionUtils      = require("utils/ExtensionUtils"),
         StringUtils         = require("utils/StringUtils");
     
-    // expermiental multi-browser implementation
-    var MultiBrowserLiveDev = require("LiveDevelopment/LiveDevMultiBrowser");
-
     var params = new UrlParams();
     var config = {
         experimental: false, // enable experimental features
@@ -75,7 +73,41 @@ define(function main(require, exports, module) {
     
     // current selected implementation (LiveDevelopment | LiveDevMultiBrowser)
     var LiveDevImpl;
+    
+    // "livedev.multibrowser" preference
+    var PREF_MULTIBROWSER = "multibrowser";
+    var prefs = PreferencesManager.getExtensionPrefs("livedev");
+    var multiBrowserPref = prefs.definePreference(PREF_MULTIBROWSER, "boolean", false);
+
+    /** Toggles or sets the preference **/
+    function _togglePref(key, value) {
+        var val,
+            oldPref = !!prefs.get(key);
 
+        if (value === undefined) {
+            val = !oldPref;
+        } else {
+            val = !!value;
+        }
+
+        // update menu
+        if (val !== oldPref) {
+            prefs.set(key, val);
+        }
+
+        return val;
+    }
+    
+    /* Toggles or sets the "livedev.multibrowser" preference */
+    function _toggleLivePreviewMultiBrowser(value) {
+        var val = _togglePref(PREF_MULTIBROWSER, value);
+        
+        CommandManager.get(Commands.TOGGLE_LIVE_PREVIEW_MB_MODE).setChecked(val);
+        // Issue #10217: multi-browser does not support user server, so disable
+        // the setting if it is enabled.
+        CommandManager.get(Commands.FILE_PROJECT_SETTINGS).setEnabled(!val);
+    }
+    
     /** Load Live Development LESS Style */
     function _loadStyles() {
         var lessText    = require("text!LiveDevelopment/main.less"),
@@ -215,10 +247,11 @@ define(function main(require, exports, module) {
         PreferencesManager.setViewState("livedev.highlight", config.highlight);
     }
     
-    // sets the MultiBrowserLiveDev implementation if multibrowser = true,
-    // keeps default LiveDevelopment implementation based on CDT in other case.
-    // since UI status are slightly different btw implementations, it also set 
-    // the corresponding style values.
+    /**
+     * Sets the MultiBrowserLiveDev implementation if multibrowser is truthy,
+     * keeps default LiveDevelopment implementation based on CDT otherwise.
+     * It also resets the listeners and UI elements.
+     */
     function _setImplementation(multibrowser) {
         if (multibrowser) {
             // set implemenation
@@ -246,6 +279,11 @@ define(function main(require, exports, module) {
                 { tooltip: Strings.LIVE_DEV_STATUS_TIP_SYNC_ERROR, style: "sync-error" }
             ];
         }
+        // setup status changes listeners for new implementation
+        _setupGoLiveButton();
+        _setupGoLiveMenu();
+        // toggle the menu
+        _toggleLivePreviewMultiBrowser(multibrowser);
     }
     
     /** Setup window references to useful LiveDevelopment modules */
@@ -261,7 +299,7 @@ define(function main(require, exports, module) {
             LiveDevelopment.reload();
         }
     }
-
+    
     /** Initialize LiveDevelopment */
     AppInit.appReady(function () {
         params.parse();
@@ -274,13 +312,12 @@ define(function main(require, exports, module) {
         // It has to be initiated at this point in case of dynamically switching 
         // by changing the preference value.
         MultiBrowserLiveDev.init(config);
-        
-        _loadStyles();
-        _setupGoLiveButton();
-        _setupGoLiveMenu();
 
+        _loadStyles();
         _updateHighlightCheckmark();
         
+        _setImplementation(prefs.get(PREF_MULTIBROWSER));
+        
         if (config.debug) {
             _setupDebugHelpers();
         }
@@ -299,6 +336,25 @@ define(function main(require, exports, module) {
                 LiveDevelopment.redrawHighlight();
             }
         });
+        
+        multiBrowserPref
+            .on("change", function () {
+                // Stop the current session if it is open and set implementation based on 
+                // the pref value. We could start the new implementation immediately, but
+                // since the current document is potentially a user preferences file, Live
+                // Preview will not locate the html file to serve.
+                if (LiveDevImpl && LiveDevImpl.status >= LiveDevImpl.STATUS_ACTIVE) {
+                    LiveDevImpl.close()
+                        .done(function () {
+                            // status changes will now be listened by the new implementation
+                            LiveDevImpl.off("statusChange");
+                            _setImplementation(prefs.get(PREF_MULTIBROWSER));
+                        });
+                } else {
+                    _setImplementation(prefs.get(PREF_MULTIBROWSER));
+                }
+            });
+
     });
     
     // init prefs
@@ -312,29 +368,15 @@ define(function main(require, exports, module) {
         "highlight": "user livedev.highlight",
         "afterFirstLaunch": "user livedev.afterFirstLaunch"
     }, true);
-    
-    PreferencesManager.definePreference("livedev.multibrowser", "boolean", false)
-        .on("change", function () {
-            // stop the current session if it is open and set implementation based on 
-            // the pref value. It could be automaticallty restarted but, since the preference file,
-            // is the document open in the editor, it will no launch a live document.
-            if (LiveDevImpl && LiveDevImpl.status >= LiveDevImpl.STATUS_ACTIVE) {
-                LiveDevImpl.close();
-                // status changes will now be listen from the new implementation
-                LiveDevImpl.off('statusChange');
-            }
-            _setImplementation(PreferencesManager.get('livedev.multibrowser'));
-            // setup status changes listeners for new implementation
-            _setupGoLiveButton();
-            _setupGoLiveMenu();
-        });
-    
+        
     config.highlight = PreferencesManager.getViewState("livedev.highlight");
    
     // init commands
     CommandManager.register(Strings.CMD_LIVE_FILE_PREVIEW,  Commands.FILE_LIVE_FILE_PREVIEW, _handleGoLiveCommand);
     CommandManager.register(Strings.CMD_LIVE_HIGHLIGHT, Commands.FILE_LIVE_HIGHLIGHT, _handlePreviewHighlightCommand);
     CommandManager.register(Strings.CMD_RELOAD_LIVE_PREVIEW, Commands.CMD_RELOAD_LIVE_PREVIEW, _handleReloadLivePreviewCommand);
+    CommandManager.register(Strings.CMD_TOGGLE_LIVE_PREVIEW_MB_MODE, Commands.TOGGLE_LIVE_PREVIEW_MB_MODE, _toggleLivePreviewMultiBrowser);
+    
     CommandManager.get(Commands.FILE_LIVE_HIGHLIGHT).setEnabled(false);
 
     // Export public functions
diff --git a/src/command/Commands.js b/src/command/Commands.js
@@ -48,6 +48,7 @@ define(function (require, exports, module) {
     exports.FILE_CLOSE_LIST             = "file.close_list";            // DocumentCommandHandlers.js   handleFileCloseList()
     exports.FILE_OPEN_DROPPED_FILES     = "file.openDroppedFiles";      // DragAndDrop.js               openDroppedFiles()
     exports.FILE_LIVE_FILE_PREVIEW      = "file.liveFilePreview";       // LiveDevelopment/main.js      _handleGoLiveCommand()
+    exports.TOGGLE_LIVE_PREVIEW_MB_MODE = "file.toggleLivePreviewMB";   // LiveDevelopment/main.js      _toggleLivePreviewMultiBrowser()
     exports.CMD_RELOAD_LIVE_PREVIEW     = "file.reloadLivePreview";     // LiveDevelopment/main.js      _handleReloadLivePreviewCommand()
     exports.FILE_LIVE_HIGHLIGHT         = "file.previewHighlight";      // LiveDevelopment/main.js      _handlePreviewHighlightCommand()
     exports.FILE_PROJECT_SETTINGS       = "file.projectSettings";       // ProjectManager.js            _projectSettings()
diff --git a/src/command/DefaultMenus.js b/src/command/DefaultMenus.js
@@ -53,6 +53,7 @@ define(function (require, exports, module) {
         menu.addMenuItem(Commands.FILE_SAVE_AS);
         menu.addMenuDivider();
         menu.addMenuItem(Commands.FILE_LIVE_FILE_PREVIEW);
+        menu.addMenuItem(Commands.TOGGLE_LIVE_PREVIEW_MB_MODE);
         menu.addMenuItem(Commands.FILE_PROJECT_SETTINGS);
         menu.addMenuDivider();
         menu.addMenuItem(Commands.FILE_EXTENSION_MANAGER);
diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js
@@ -317,6 +317,7 @@ define({
     "CMD_FILE_SAVE_ALL"                   : "Save All",
     "CMD_FILE_SAVE_AS"                    : "Save As\u2026",
     "CMD_LIVE_FILE_PREVIEW"               : "Live Preview",
+    "CMD_TOGGLE_LIVE_PREVIEW_MB_MODE"     : "Enable Experimental Live Preview",
     "CMD_RELOAD_LIVE_PREVIEW"             : "Force Reload Live Preview",
     "CMD_PROJECT_SETTINGS"                : "Project Settings\u2026",
     "CMD_FILE_RENAME"                     : "Rename",
diff --git a/test/spec/LiveDevelopmentMultiBrowser-test.js b/test/spec/LiveDevelopmentMultiBrowser-test.js
@@ -28,7 +28,7 @@ define(function (require, exports, module) {
     
     var SpecRunnerUtils = require("spec/SpecRunnerUtils");
 
-    describe("MultiBrowser (experimental) - LiveDevelopment", function () {
+    describe("MultiBrowser (experimental)", function () {
     
         this.category = "livepreview";
 
@@ -66,6 +66,7 @@ define(function (require, exports, module) {
         });
         
         afterEach(function () {
+            LiveDevelopment.close();
             SpecRunnerUtils.closeTestWindow();
             testWindow = null;
             brackets = null;
@@ -88,7 +89,6 @@ define(function (require, exports, module) {
 
         describe("Init Session", function () {
             
-            
             it("should establish a browser connection for an opened html file", function () {
                 //open a file
                 runs(function () {

Original file line number	Diff line number	Diff line change
`@@ -467,10 +467,13 @@ define(function (require, exports, module) {`
`467`	`467`	`}`
`468`	`468`
`469`	`469`	`/**`
`470`		`- * Close all active connections`
	`470`	`+ * Closes all active connections.`
	`471`	`+ * Returns a resolved promise for API compatibility.`
	`472`	`+ * @return {$.Promise} A resolved promise`
`471`	`473`	`*/`
`472`	`474`	`function close() {`
`473`		`- return _close(true);`
	`475`	`+ _close(true);`
	`476`	`+ return new $.Deferred().resolve().promise();`
`474`	`477`	`}`
`475`	`478`
`476`	`479`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,7 @@`
`52`	`52`	`* 2: Loading agents`
`53`	`53`	`* 3: Active`
`54`	`54`	`* 4: Out of sync`
	`55`	`+ * 5: Sync error`
`55`	`56`	`*`
`56`	`57`	`* The reason codes are:`
`57`	`58`	`* - null (Unknown reason)`