From cd56e7fad14fb878d20066310f16a96d4b6395f6 Mon Sep 17 00:00:00 2001 From: Swagatam Mitra Date: Thu, 5 May 2016 12:38:20 +0530 Subject: [PATCH 1/5] Fix for documents refCount issue --- src/document/Document.js | 28 ++++++++++++++++++++++------ src/editor/EditorManager.js | 13 ++++++++----- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/document/Document.js b/src/document/Document.js index 97700a313b2..f054ad95471 100644 --- a/src/document/Document.js +++ b/src/document/Document.js @@ -200,16 +200,14 @@ define(function (require, exports, module) { * @param {!Editor} masterEditor */ Document.prototype._makeEditable = function (masterEditor) { - if (this._masterEditor) { - //Already a master editor is associated , so preserve the old editor in list of full editors - if (this._associatedFullEditors.indexOf(this._masterEditor) < 0) { - this._associatedFullEditors.push(this._masterEditor); - } - } this._text = null; this._masterEditor = masterEditor; + if (this._associatedFullEditors.indexOf(this._masterEditor) < 0) { + this._associatedFullEditors.push(this._masterEditor); + } + masterEditor.on("change", this._handleEditorChange.bind(this)); }; @@ -252,6 +250,24 @@ define(function (require, exports, module) { } }; + + /** + * Checks and returns if a full editor exists for the provided pane attached to this document + * @param {String} paneId + * @return {Editor} Attached editor bound to the provided pane id + */ + Document.prototype._checkAssociatedEditorForPane = function (paneId) { + var editorCount, editorForPane; + for (editorCount = 0; editorCount < this._associatedFullEditors; editorCount++) { + if (this._associatedFullEditors[editorCount]._paneId === paneId) { + editorForPane = this._associatedFullEditors[editorCount]; + break; + } + } + + return editorForPane; + }; + /** * Disassociates an editor from this document if present in the associated editor list * To be used internally by Editor only when destroyed and not the current master editor for the document diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index e8a12db9ed7..d9fff3789ba 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -541,11 +541,14 @@ define(function (require, exports, module) { function _showEditor(document, pane, editorOptions) { // Ensure a main editor exists for this document to show in the UI var createdNewEditor = false, - editor = document._masterEditor; - - //Check if a master editor is not set already or the current master editor doesn't belong - //to the pane container requested - to support creation of multiple full editors - if (!editor || editor._paneId !== pane.id) { + // Check if any full editor is present marked for the requested pane + // This check is required as _masterEditor is the active full editor for the document + // provided and there can be existing full editor created for other panes + editor = document._checkAssociatedEditorForPane(pane.id) || document._masterEditor; + + // Check if a master editor is not set already or the current master editor doesn't belong + // to the pane container requested - to support creation of multiple full editors + if (!editor || (editor._paneId && editor._paneId !== pane.id)) { // Performance (see #4757) Chrome wastes time messing with selection // that will just be changed at end, so clear it for now if (window.getSelection && window.getSelection().empty) { // Chrome From 1a259bb4331865309caf6ad5681de5d970176659 Mon Sep 17 00:00:00 2001 From: Swagatam Mitra Date: Thu, 5 May 2016 19:01:08 +0530 Subject: [PATCH 2/5] Missing length --- src/document/Document.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/document/Document.js b/src/document/Document.js index f054ad95471..03ba85199a1 100644 --- a/src/document/Document.js +++ b/src/document/Document.js @@ -258,7 +258,7 @@ define(function (require, exports, module) { */ Document.prototype._checkAssociatedEditorForPane = function (paneId) { var editorCount, editorForPane; - for (editorCount = 0; editorCount < this._associatedFullEditors; editorCount++) { + for (editorCount = 0; editorCount < this._associatedFullEditors.length; editorCount++) { if (this._associatedFullEditors[editorCount]._paneId === paneId) { editorForPane = this._associatedFullEditors[editorCount]; break; From 7990632650cc67bf664800de433ab827f681f2e6 Mon Sep 17 00:00:00 2001 From: Swagatam Mitra Date: Tue, 17 May 2016 17:02:44 +0530 Subject: [PATCH 3/5] Doc ref count fix --- src/document/Document.js | 30 ++++++++++++++---------------- src/editor/Editor.js | 21 +++++++++++++-------- src/editor/EditorManager.js | 13 ++++++++----- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/document/Document.js b/src/document/Document.js index 03ba85199a1..9abcd6205db 100644 --- a/src/document/Document.js +++ b/src/document/Document.js @@ -79,15 +79,12 @@ define(function (require, exports, module) { this.file = file; this._updateLanguage(); this.refreshText(rawText, initialTimestamp, true); + // List of full editors which are initialized as master editors for this doc. + this._associatedFullEditors = []; } EventDispatcher.makeEventDispatcher(Document.prototype); - /** - * List of editors which were initialized as master editors for this doc. - */ - Document.prototype._associatedFullEditors = []; - /** * Number of clients who want this Document to stay alive. The Document is listed in * DocumentManager._openDocuments whenever refCount > 0. @@ -204,10 +201,6 @@ define(function (require, exports, module) { this._text = null; this._masterEditor = masterEditor; - if (this._associatedFullEditors.indexOf(this._masterEditor) < 0) { - this._associatedFullEditors.push(this._masterEditor); - } - masterEditor.on("change", this._handleEditorChange.bind(this)); }; @@ -239,13 +232,7 @@ define(function (require, exports, module) { */ Document.prototype._toggleMasterEditor = function (masterEditor) { // Do a check before processing the request to ensure inline editors are not being set as master editor - if (this._associatedFullEditors.indexOf(masterEditor) >= 0) { - if (this._masterEditor) { - // Already a master editor is associated , so preserve the old editor in list of editors - if (this._associatedFullEditors.indexOf(this._masterEditor) < 0) { - this._associatedFullEditors.push(this._masterEditor); - } - } + if (this.file === masterEditor.document.file && this._associatedFullEditors.indexOf(masterEditor) >= 0) { this._masterEditor = masterEditor; } }; @@ -279,6 +266,17 @@ define(function (require, exports, module) { } }; + /** + * Aassociates a full editor to this document + * To be used internally by Editor only when pane marking happens + */ + Document.prototype._associateEditor = function (editor) { + // Do a check before processing the request to ensure inline editors are not being handled + if (this._associatedFullEditors.indexOf(editor) === -1) { + this._associatedFullEditors.push(editor); + } + }; + /** * Guarantees that _masterEditor is non-null. If needed, asks EditorManager to create a new master * editor bound to this Document (which in turn causes Document._makeEditable() to be called). diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 6144c043a7a..7cf92d98beb 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -428,6 +428,15 @@ define(function (require, exports, module) { this.on("change", function (event, editor, changeList) { self._handleEditorChange(changeList); }); + this.on("focus", function (event, editor) { + if (self._hostEditor) { + // Mark the host editor as the master editor for the hosting document + self._hostEditor.document._toggleMasterEditor(self._hostEditor); + } else { + // Set this full editor as master editor for the document + self.document._toggleMasterEditor(self); + } + }); // Set code-coloring mode BEFORE populating with text, to avoid a flash of uncolored text this._codeMirror.setOption("mode", mode); @@ -468,6 +477,10 @@ define(function (require, exports, module) { Editor.prototype.markPaneId = function (paneId) { this._paneId = paneId; + + // Also add this to the pool of full editors + this.document._associateEditor(this); + // In case this Editor is initialized not as the first full editor for the document // and the document is already dirty and present in another working set, make sure // to add this documents to the new panes working set. @@ -993,14 +1006,6 @@ define(function (require, exports, module) { // Convert CodeMirror onFocus events to EditorManager activeEditorChanged this._codeMirror.on("focus", function () { - if (self._hostEditor) { - // Mark the host editor as the master editor for the hosting document - self._hostEditor.document._toggleMasterEditor(self._hostEditor); - } else { - // Set this full editor as master editor for the document - self.document._toggleMasterEditor(self); - } - self._focused = true; self.trigger("focus", self); diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index d9fff3789ba..9de7909ab0a 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -541,14 +541,17 @@ define(function (require, exports, module) { function _showEditor(document, pane, editorOptions) { // Ensure a main editor exists for this document to show in the UI var createdNewEditor = false, - // Check if any full editor is present marked for the requested pane - // This check is required as _masterEditor is the active full editor for the document - // provided and there can be existing full editor created for other panes - editor = document._checkAssociatedEditorForPane(pane.id) || document._masterEditor; + editor = document._masterEditor; // Check if a master editor is not set already or the current master editor doesn't belong // to the pane container requested - to support creation of multiple full editors - if (!editor || (editor._paneId && editor._paneId !== pane.id)) { + // This check is required as _masterEditor is the active full editor for the document + // and there can be existing full editor created for other panes + if (editor && editor._paneId && editor._paneId !== pane.id) { + editor = document._checkAssociatedEditorForPane(pane.id); + } + + if (!editor) { // Performance (see #4757) Chrome wastes time messing with selection // that will just be changed at end, so clear it for now if (window.getSelection && window.getSelection().empty) { // Chrome From b21ce25d4f947e26cd720984547459128aba56c7 Mon Sep 17 00:00:00 2001 From: Swagatam Mitra Date: Wed, 18 May 2016 16:10:30 +0530 Subject: [PATCH 4/5] Putting back missing ckeck and add to pane views --- src/editor/EditorManager.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index 9de7909ab0a..2b1f4d53f39 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -561,6 +561,10 @@ define(function (require, exports, module) { // Editor doesn't exist: populate a new Editor with the text editor = _createFullEditorForDocument(document, pane, editorOptions); createdNewEditor = true; + } else if (editor.$el.parent()[0] !== pane.$content[0]) { + // editor does exist but is not a child of the pane so add it to the + // pane (which will switch the view's container as well) + pane.addView(editor); } // show the view From 0c8b2cd36334553a2053708661a1ef01291d82c4 Mon Sep 17 00:00:00 2001 From: Prashanth Nethi Date: Thu, 19 May 2016 18:33:00 +0530 Subject: [PATCH 5/5] Update Document.js Changing `editorCount++` to `++editorCount` --- src/document/Document.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/document/Document.js b/src/document/Document.js index 9abcd6205db..433b2dec688 100644 --- a/src/document/Document.js +++ b/src/document/Document.js @@ -245,7 +245,7 @@ define(function (require, exports, module) { */ Document.prototype._checkAssociatedEditorForPane = function (paneId) { var editorCount, editorForPane; - for (editorCount = 0; editorCount < this._associatedFullEditors.length; editorCount++) { + for (editorCount = 0; editorCount < this._associatedFullEditors.length; ++editorCount) { if (this._associatedFullEditors[editorCount]._paneId === paneId) { editorForPane = this._associatedFullEditors[editorCount]; break;