Merge remote-tracking branch 'origin/master' into tvoliter/issue-559

* origin/master:
  Code review comments
  Mix merge midair collision from API rename
  More code review changes: - Rename CSSAgent methods for clarity - When discarding unsaved changes, revert Document contents after closing   main editor instead of before. As a bonus, this makes it easy to skip the   revert if the main editor was the only view attached to the Document.
  Respond to code review: - unregister LiveDevelopment's listeners on CSSDocument when LiveDevelopment   stops caring about a particular document - fix misnamed function
  - Remove the isDirty assertion check in Document._makeNonEditable(). This was breaking many unit tests, which frequently throw away unsaved Documents. It also required somewhat hacky code in notifyFileDeleted() to avoid hitting the assertion in that case. The assertion was sort of out of place anyway: there's nothing intrinsic about destroying _masterEditor and moving the text back into a string that makes unsaved changes not ok; the idea that unsaved changes should be dealt with before closing the main editor is really just a UI feature/rule.
  When main editor is closed without saving changes, revert the Document to whatever's on disk - because other parts of the UI might still be showing the contents of the Document, and we don't want them to keep reflecting the unsaved (now discarded) changes.
  Change CSSDocument so it doesn't know about LiveDevelopment, per discussion with Glenn. When a CSSDocument gets a Document "deleted" event, it now dispatches another "deleted" event of its own rather than pinging LiveDevelopment directly to unregister itself; LiveDevelopment is now responsible for listening for this CSSDocument event and unregistering the CSSDocument automatically.
  - Support CSS files being deleted in live development (now updates browser   accordingly) - Fix exception in Document when unsaved changes are discarded but live   development still has a ref to the doc (bug #546). More comprehensive fix   probably to follow, though - Add warning in DocumentManager when deleting a doc does not clear all refs
  Fix for #532 and #537

src/LiveDevelopment/Agents/CSSAgent.js

@@ -54,11 +54,20 @@ define(function CSSAgent(require, exports, module) {
     /** Reload a CSS style sheet from a document
      * @param {Document} document
      */
-    function reloadDocument(doc) {
+    function reloadCSSForDocument(doc) {
         var style = styleForURL(doc.url);
         console.assert(style, "Style Sheet for document not loaded: " + doc.url);
         Inspector.CSS.setStyleSheetText(style.styleSheetId, doc.getText());
     }
+
+    /** Empties a CSS style sheet given a document that has been deleted
+     * @param {Document} document
+     */
+    function clearCSSForDocument(doc) {
+        var style = styleForURL(doc.url);
+        console.assert(style, "Style Sheet for document not loaded: " + doc.url);
+        Inspector.CSS.setStyleSheetText(style.styleSheetId, "");
+    }
 
     /** Initialize the agent */
     function load() {
@@ -75,7 +84,8 @@ define(function CSSAgent(require, exports, module) {
     // Export public functions
     exports.styleForURL = styleForURL;
     exports.getStylesheetURLs = getStylesheetURLs;
-    exports.reloadDocument = reloadDocument;
+    exports.reloadCSSForDocument = reloadCSSForDocument;
+    exports.clearCSSForDocument = clearCSSForDocument;
     exports.load = load;
     exports.unload = unload;
 });

src/LiveDevelopment/Documents/CSSDocument.js

@@ -19,6 +19,12 @@
  * CSSDocument supports highlighting nodes from the HighlightAgent and
  * highlighting all DOMNode corresponding to the rule at the cursor position
  * in the editor.
+ *
+ * # EVENTS
+ *
+ * CSSDocument dispatches these events:
+ *  deleted - When the file for the underlying Document has been deleted. The
+ *      2nd argument to the listener will be this CSSDocument.
  */
 define(function CSSDocumentModule(require, exports, module) {
     'use strict';
@@ -47,7 +53,9 @@ define(function CSSDocumentModule(require, exports, module) {
         // Add a ref to the doc since we're listening for change events
         this.doc.addRef();
         this.onChange = this.onChange.bind(this);
+        this.onDeleted = this.onDeleted.bind(this);
         $(this.doc).on("change", this.onChange);
+        $(this.doc).on("deleted", this.onDeleted);
 
 /*
         $(this.editor).on("cursorActivity", this.onCursorActivity);
@@ -65,13 +73,14 @@ define(function CSSDocumentModule(require, exports, module) {
 
         // If the CSS document is dirty, push the changes into the browser now
         if (doc.isDirty) {
-            CSSAgent.reloadDocument(this.doc);
+            CSSAgent.reloadCSSForDocument(this.doc);
         }
     };
 
     /** Close the document */
     CSSDocument.prototype.close = function close() {
         $(this.doc).off("change", this.onChange);
+        $(this.doc).off("deleted", this.onDeleted);
         this.doc.releaseRef();
 /*
         Inspector.off("HighlightAgent.highlight", this.onHighlight);
@@ -109,10 +118,19 @@ define(function CSSDocumentModule(require, exports, module) {
         }
     };
 
-    /** Triggered on change of the editor */
+    /** Triggered whenever the Document is edited */
     CSSDocument.prototype.onChange = function onChange(event, editor, change) {
         // brute force: update the CSS
-        CSSAgent.reloadDocument(this.doc);
+        CSSAgent.reloadCSSForDocument(this.doc);
+    };
+    /** Triggered if the Document's file is deleted */
+    CSSDocument.prototype.onDeleted = function onDeleted(event, editor, change) {
+        // clear the CSS
+        CSSAgent.clearCSSForDocument(this.doc);
+
+        // shut down, since our Document is now dead
+        this.close();
+        $(this).triggerHandler("deleted", [this]);
     };
 
     /** Triggered by the HighlightAgent to highlight a node in the editor */

src/LiveDevelopment/LiveDevelopment.js

@@ -133,11 +133,25 @@ define(function LiveDevelopment(require, exports, module) {
         if (_relatedDocuments) {
             _relatedDocuments.forEach(function (liveDoc) {
                 liveDoc.close();
+                $(liveDoc).off("deleted", _handleRelatedDocumentDeleted);
             });
             _relatedDocuments = undefined;
         }
     }
 
+    /**
+     * Removes the given CSS/JSDocument from _relatedDocuments. Signals that the
+     * given file is no longer associated with the HTML document that is live (e.g.
+     * if the related file has been deleted on disk).
+     */
+    function _handleRelatedDocumentDeleted(event, liveDoc) {
+        var index = _relatedDocuments.indexOf(liveDoc);
+        if (index !== -1) {
+            $(liveDoc).on("deleted", _handleRelatedDocumentDeleted);
+            _relatedDocuments.splice(index, 1);
+        }
+    }
+
     /** Create a live version of a Brackets document */
     function _createDocument(doc, editor) {
         var DocClass = _classForDocument(doc);
@@ -179,6 +193,7 @@ define(function LiveDevelopment(require, exports, module) {
                     var liveDoc = _createDocument(doc);
                     if (liveDoc) {
                         _relatedDocuments.push(liveDoc);
+                        $(liveDoc).on("deleted", _handleRelatedDocumentDeleted);
                     }
                 });
         });

src/document/DocumentCommandHandlers.js

@@ -338,6 +338,31 @@ define(function (require, exports, module) {
         );
     }
 
+    /**
+     * Reverts the Document to the current contents of its file on disk. Discards any unsaved changes
+     * in the Document.
+     * @param {Document} doc
+     * @return {$.Promise} a Promise that's resolved when done, or rejected with a FileError if the
+     *      file cannot be read (after showing an error dialog to the user).
+     */
+    function doRevert(doc) {
+        var result = new $.Deferred();
+
+        FileUtils.readAsText(doc.file)
+            .done(function (text, readTimestamp) {
+                doc.refreshText(text, readTimestamp);
+                result.resolve();
+            })
+            .fail(function (error) {
+                FileUtils.showFileOpenError(error.code, doc.file.fullPath)
+                    .always(function () {
+                        result.reject(error);
+                    });
+            });
+
+        return result.promise();
+    }
+
 
     /**
      * Closes the specified file: removes it from the working set, and closes the main editor if one
@@ -395,6 +420,7 @@ define(function (require, exports, module) {
                 if (id === Dialogs.DIALOG_BTN_CANCEL) {
                     result.reject();
                 } else if (id === Dialogs.DIALOG_BTN_OK) {
+                    // "Save" case: wait until we confirm save has succeeded before closing
                     doSave(doc)
                         .done(function () {
                             doClose(file);
@@ -404,13 +430,18 @@ define(function (require, exports, module) {
                             result.reject();
                         });
                 } else {
-                    // This is the "Don't Save" case--we can just go ahead and close the file.
-                    // FUTURE: If other views of the Document will remain open, we need to revert the
-                    // Document to the clean content on disk. Currently secondary Editors all lose
-                    // sync & close if you refresh the whole doc's text, so we just fake this rather
-                    // than pointlessly load text from disk. See hack in Document._makeNonEditable().
+                    // "Don't Save" case: even though we're closing the main editor, other views of
+                    // the Document may remain in the UI. So we need to revert the Document to a clean
+                    // copy of whatever's on disk.
                     doClose(file);
-                    result.resolve();
+
+                    // Only reload from disk if other views still exist
+                    if (DocumentManager.getOpenDocumentForPath(file.fullPath)) {
+                        doRevert(doc)
+                            .pipe(result.resolve, result.reject);
+                    } else {
+                        result.resolve();
+                    }
                 }
             });
             result.always(function () {

src/document/DocumentManager.js

@@ -440,15 +440,6 @@ define(function (require, exports, module) {
         } else {
             this._text = this.getText();
             this._masterEditor = null;
-
-            // FUTURE: If main editor was closed without saving changes, we should revert _text to
-            // what's on disk. But since we currently close all secondary editors when anyone else
-            // touches the Document content, there's no point in doing that yet. Just change the text
-            // to a dummy value to trigger that closing. Ultimately, the nicer "revert" behavior
-            // should probably live in DocumentCommandHandlers.handleFileClose() (see note there).
-            if (this.isDirty) {
-                this.refreshText("");
-            }
         }
     };
 
@@ -644,6 +635,11 @@ define(function (require, exports, module) {
         if (doc) {
             $(doc).triggerHandler("deleted");
         }
+
+        // At this point, all those other views SHOULD have released the Doc
+        if (doc && doc._refCount > 0) {
+            console.log("WARNING: deleted Document still has " + doc._refCount + " references. Did someone addRef() without listening for 'deleted'?");
+        }
     }
 
 

src/editor/CSSInlineEditor.js

@@ -50,7 +50,6 @@ define(function (require, exports, module) {
 
         // Container to hold all editors
         var self = this,
-            hostScroller = hostEditor._codeMirror.getScrollerElement(),
             $ruleItem,
             $location;
 
@@ -63,9 +62,9 @@ define(function (require, exports, module) {
 
         // Outer container for border-left and scrolling
         this.$relatedContainer = $(document.createElement("div")).addClass("relatedContainer");
-        // Position immediately to the left of the main editor's scrollbar.
-        var rightOffset = $(document.body).outerWidth() - ($(hostScroller).offset().left + $(hostScroller).get(0).clientWidth);
-        this.$relatedContainer.css("right", rightOffset + "px");
+        this._relatedContainerInserted = false;
+        this._relatedContainerInsertedHandler = this._relatedContainerInsertedHandler.bind(this);
+        this.$relatedContainer.on("DOMNodeInserted", this._relatedContainerInsertedHandler);
 
         // List "selection" highlight
         this.$selectedMarker = $(document.createElement("div")).appendTo(this.$relatedContainer).addClass("selection");
@@ -110,6 +109,10 @@ define(function (require, exports, module) {
         // Listen to the editor's scroll event to reposition the relatedContainer.
         $(this.hostEditor).on("scroll", this._updateRelatedContainer);
 
+        // Listen to the window resize event to reposition the relatedContainer
+        // when the hostEditor's scrollbars visibility changes
+        $(window).on("resize", this._updateRelatedContainer);
+
         // Listen for clicks directly on us, so we can set focus back to the editor
         this.$htmlContent.on("click", this._onClick);
 
@@ -195,6 +198,16 @@ define(function (require, exports, module) {
         $(this.hostEditor).off("change", this._updateRelatedContainer);
         $(this.editors[0]).off("change", this._updateRelatedContainer);
         $(this.hostEditor).off("scroll", this._updateRelatedContainer);
+        $(window).off("resize", this._updateRelatedContainer);
+    };
+
+    /**
+     * @private
+     * Set _relatedContainerInserted flag once the $relatedContainer is inserted in the DOM.
+     */
+    CSSInlineEditor.prototype._relatedContainerInsertedHandler = function () {
+        this.$relatedContainer.off("DOMNodeInserted", this._relatedContainerInsertedHandler);
+        this._relatedContainerInserted = true;
     };
 
     /**
@@ -230,14 +243,32 @@ define(function (require, exports, module) {
             rcTop = this.$relatedContainer.offset().top,
             rcHeight = this.$relatedContainer.outerHeight(),
             rcBottom = rcTop + rcHeight,
-            scrollerTop = $(hostScroller).offset().top,
-            scrollerBottom = scrollerTop + hostScroller.clientHeight;
+            scrollerOffset = $(hostScroller).offset(),
+            scrollerTop = scrollerOffset.top,
+            scrollerBottom = scrollerTop + hostScroller.clientHeight,
+            scrollerLeft = scrollerOffset.left,
+            rightOffset = $(document.body).outerWidth() - (scrollerLeft + hostScroller.clientWidth);
         if (rcTop < scrollerTop || rcBottom > scrollerBottom) {
             this.$relatedContainer.css("clip", "rect(" + Math.max(scrollerTop - rcTop, 0) + "px, auto, " +
                                        (rcHeight - Math.max(rcBottom - scrollerBottom, 0)) + "px, auto)");
         } else {
             this.$relatedContainer.css("clip", "");
         }
+
+        // Constrain relatedContainer width to half of the scroller width
+        var relatedContainerWidth = this.$relatedContainer.width();
+        if (this._relatedContainerInserted) {
+            if (this._relatedContainerDefaultWidth === undefined) {
+                this._relatedContainerDefaultWidth = relatedContainerWidth;
+            }
+
+            var halfWidth = Math.floor(hostScroller.clientWidth / 2);
+            relatedContainerWidth = Math.min(this._relatedContainerDefaultWidth, halfWidth);
+            this.$relatedContainer.width(relatedContainerWidth);
+        }
+
+        // Position immediately to the left of the main editor's scrollbar.
+        this.$relatedContainer.css("right", rightOffset + "px");
     };
 
     /**

test/spec/InlineEditorProviders-test.js

@@ -528,7 +528,7 @@ define(function (require, exports, module) {
                 });
 
                 // FUTURE: Eventually we'll instead want it to stay open and revert to the content on disk.
-                // See notes in Document._makeNonEditable() & DocumentCommandHandlers.handleFileClose().
+                // Editor's syncing code can't yet handle blowing away the whole Document like that, though.
                 it("should close inline when file is closed without saving changes", function () {
                     initInlineTest("test1.html", 1);