Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix for documents refCount issue #12405

Merged
merged 5 commits into from
May 19, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions src/document/Document.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
};

Expand Down Expand Up @@ -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++) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing length there: editorCount < this._associatedFullEditors.length 🔥

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. Issue with copy paste from another repo. Fixed now.

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
Expand Down
13 changes: 8 additions & 5 deletions src/editor/EditorManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down