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

Commit efbd1dc

Browse files
RaymondLimRaymondLim
committed
Merge pull request #6002 from adobe/couzteau/fix-5986
Fix Unit tests, Move async file.exists test out of EditorManager into DocumentCommandHandlers
2 parents 30eccd5 + 0aba29b commit efbd1dc

File tree

3 files changed

+117
-131
lines changed

3 files changed

+117
-131
lines changed

‎src/document/DocumentCommandHandlers.js‎

Lines changed: 50 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ define(function (require, exports, module) {
3636
DocumentManager = require("document/DocumentManager"),
3737
EditorManager = require("editor/EditorManager"),
3838
FileSystem = require("filesystem/FileSystem"),
39+
FileSystemError = require("filesystem/FileSystemError"),
3940
FileUtils = require("file/FileUtils"),
4041
FileViewController = require("project/FileViewController"),
4142
InMemoryFile = require("document/InMemoryFile"),
@@ -178,7 +179,43 @@ define(function (require, exports, module) {
178179
*/
179180
function doOpen(fullPath, silent) {
180181
var result = new $.Deferred();
181-
182+
183+
// workaround for https://github.com/adobe/brackets/issues/6001
184+
// TODO should be removed once bug is closed.
185+
// if we are already displaying a file do nothing but resolve immediately.
186+
// this fixes timing issues in test cases.
187+
if (EditorManager.getCurrentlyViewedPath() === fullPath) {
188+
result.resolve(DocumentManager.getCurrentDocument());
189+
return result.promise();
190+
}
191+
192+
function _cleanup(fullFilePath) {
193+
if (!fullFilePath || EditorManager.showingCustomViewerForPath(fullFilePath)) {
194+
// We get here only after the user renames a file that makes it no longer belong to a
195+
// custom viewer but the file is still showing in the current custom viewer. This only
196+
// occurs on Mac since opening a non-text file always fails on Mac and triggers an error
197+
// message that in turn calls _cleanup() after the user clicks OK in the message box.
198+
// So we need to explicitly close the currently viewing image file whose filename is
199+
// no longer valid. Calling notifyPathDeleted will close the image vieer and then select
200+
// the previously opened text file or show no-editor if none exists.
201+
EditorManager.notifyPathDeleted(fullFilePath);
202+
} else {
203+
// For performance, we do lazy checking of file existence, so it may be in working set
204+
DocumentManager.removeFromWorkingSet(FileSystem.getFileForPath(fullFilePath));
205+
EditorManager.focusEditor();
206+
}
207+
result.reject();
208+
}
209+
function _showErrorAndCleanUp(fileError, fullFilePath) {
210+
if (silent) {
211+
_cleanup(fullFilePath);
212+
} else {
213+
FileUtils.showFileOpenError(fileError, fullFilePath).done(function () {
214+
_cleanup(fullFilePath);
215+
});
216+
}
217+
}
218+
182219
if (!fullPath) {
183220
console.error("doOpen() called without fullPath");
184221
result.reject();
@@ -190,8 +227,17 @@ define(function (require, exports, module) {
190227

191228
var viewProvider = EditorManager.getCustomViewerForPath(fullPath);
192229
if (viewProvider) {
193-
EditorManager.showCustomViewer(viewProvider, fullPath);
194-
result.resolve();
230+
var file = FileSystem.getFileForPath(fullPath);
231+
file.exists(function (fileError, fileExists) {
232+
if (fileExists) {
233+
EditorManager.showCustomViewer(viewProvider, fullPath);
234+
result.resolve();
235+
} else {
236+
fileError = fileError || FileSystemError.NOT_FOUND;
237+
_showErrorAndCleanUp(fileError);
238+
}
239+
});
240+
195241
} else {
196242
// Load the file if it was never open before, and then switch to it in the UI
197243
DocumentManager.getDocumentForPath(fullPath)
@@ -200,29 +246,7 @@ define(function (require, exports, module) {
200246
result.resolve(doc);
201247
})
202248
.fail(function (fileError) {
203-
function _cleanup() {
204-
if (EditorManager.showingCustomViewerForPath(fullPath)) {
205-
// We get here only after the user renames a file that makes it no longer belong to a
206-
// custom viewer but the file is still showing in the current custom viewer. This only
207-
// occurs on Mac since opening a non-text file always fails on Mac and triggers an error
208-
// message that in turn calls _cleanup() after the user clicks OK in the message box.
209-
// So we need to explicitly close the currently viewing image file whose filename is
210-
// no longer valid. Calling notifyPathDeleted will close the image vieer and then select
211-
// the previously opened text file or show no-editor if none exists.
212-
EditorManager.notifyPathDeleted(fullPath);
213-
} else {
214-
// For performance, we do lazy checking of file existence, so it may be in working set
215-
DocumentManager.removeFromWorkingSet(FileSystem.getFileForPath(fullPath));
216-
EditorManager.focusEditor();
217-
}
218-
result.reject();
219-
}
220-
221-
if (silent) {
222-
_cleanup();
223-
} else {
224-
FileUtils.showFileOpenError(fileError, fullPath).done(_cleanup);
225-
}
249+
_showErrorAndCleanUp(fileError, fullPath);
226250
});
227251
}
228252
}

‎src/editor/EditorManager.js‎

Lines changed: 51 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,7 @@ define(function (require, exports, module) {
6060
InlineTextEditor = require("editor/InlineTextEditor").InlineTextEditor,
6161
ImageViewer = require("editor/ImageViewer"),
6262
Strings = require("strings"),
63-
LanguageManager = require("language/LanguageManager"),
64-
FileSystem = require("filesystem/FileSystem"),
65-
FileSystemError = require("filesystem/FileSystemError"),
66-
FileUtils = require("file/FileUtils");
63+
LanguageManager = require("language/LanguageManager");
6764

6865
/** @type {jQueryObject} DOM node that contains all editors (visible and hidden alike) */
6966
var _editorHolder = null;
@@ -81,8 +78,6 @@ define(function (require, exports, module) {
8178
var _$currentCustomViewer = null;
8279
/** @type {?Object} view provider */
8380
var _currentViewProvider = null;
84-
/** Helper function defined as var to satisfy JSLint order constraints */
85-
var _checkFileExists;
8681

8782
/**
8883
* Currently focused Editor (full-size, inline, or otherwise)
@@ -630,70 +625,13 @@ define(function (require, exports, module) {
630625

631626
/** Remove existing custom view if present */
632627
function _removeCustomViewer() {
633-
window.removeEventListener("focus", _checkFileExists);
634628
$(exports).triggerHandler("removeCustomViewer");
635629
if (_$currentCustomViewer) {
636630
_$currentCustomViewer.remove();
637631
}
638632
_$currentCustomViewer = null;
639633
_currentViewProvider = null;
640634
}
641-
642-
/**
643-
* Clears custom viewer for a file with a given path and displays
644-
* an alternate file or the no editor view.
645-
* If no param fullpath is passed an alternate file will be opened
646-
* regardless of the current value of _currentlyViewedPath.
647-
* If param fullpath is provided then only if fullpath matches
648-
* the currently viewed file an alternate file will be opened.
649-
* @param {?string} fullPath - file path of deleted file.
650-
*/
651-
function notifyPathDeleted(fullPath) {
652-
function openAlternateFile() {
653-
var fileToOpen = DocumentManager.getNextPrevFile(1);
654-
if (fileToOpen) {
655-
CommandManager.execute(Commands.FILE_OPEN, {fullPath: fileToOpen.fullPath});
656-
} else {
657-
_removeCustomViewer();
658-
_showNoEditor();
659-
_setCurrentlyViewedPath();
660-
}
661-
}
662-
if (!fullPath || _currentlyViewedPath === fullPath) {
663-
openAlternateFile();
664-
}
665-
}
666-
667-
/*
668-
* show a generic error or File Not Found in modal error dialog
669-
*/
670-
function _showErrorAndNotify(err, fullPath) {
671-
var errorToShow = err || FileSystemError.NOT_FOUND;
672-
FileUtils.showFileOpenError(errorToShow, fullPath).done(
673-
function () {
674-
notifyPathDeleted();
675-
}
676-
);
677-
}
678-
679-
/*
680-
* callback function passed to file.exists. If file in view does
681-
* not exist the current view will be replaced.
682-
*/
683-
function _removeViewIfFileDeleted(err, fileExists) {
684-
if (!fileExists) {
685-
notifyPathDeleted();
686-
}
687-
}
688-
689-
/**
690-
* Makes sure that the file in view is present in the file system
691-
* Close and warn if file is gone.
692-
*/
693-
_checkFileExists = function () {
694-
var file = FileSystem.getFileForPath(getCurrentlyViewedPath());
695-
file.exists(_removeViewIfFileDeleted);
696-
};
697635

698636
/**
699637
* Closes the customViewer currently displayed, shows the NoEditor view
@@ -704,50 +642,39 @@ define(function (require, exports, module) {
704642
_setCurrentlyViewedPath();
705643
_showNoEditor();
706644
}
707-
645+
708646
/**
709647
* Append custom view to editor-holder
710648
* @param {!Object} provider custom view provider
711649
* @param {!string} fullPath path to the file displayed in the custom view
712650
*/
713651
function showCustomViewer(provider, fullPath) {
714-
function _doShow(err, fileExists) {
715-
if (!fileExists) {
716-
_showErrorAndNotify(err, fullPath);
717-
} else {
718-
// Don't show the same custom view again if file path
719-
// and view provider are still the same.
720-
if (_currentlyViewedPath === fullPath &&
721-
_currentViewProvider === provider) {
722-
return;
723-
}
724-
725-
// Clean up currently viewing document or custom viewer
726-
DocumentManager._clearCurrentDocument();
727-
_removeCustomViewer();
728-
729-
// Hide the not-editor or reset current editor
730-
$("#not-editor").css("display", "none");
731-
_nullifyEditor();
652+
// Don't show the same custom view again if file path
653+
// and view provider are still the same.
654+
if (_currentlyViewedPath === fullPath &&
655+
_currentViewProvider === provider) {
656+
return;
657+
}
732658

733-
_currentViewProvider = provider;
734-
_$currentCustomViewer = provider.getCustomViewHolder(fullPath);
659+
// Clean up currently viewing document or custom viewer
660+
DocumentManager._clearCurrentDocument();
661+
_removeCustomViewer();
662+
663+
// Hide the not-editor or reset current editor
664+
$("#not-editor").css("display", "none");
665+
_nullifyEditor();
666+
667+
_currentViewProvider = provider;
668+
_$currentCustomViewer = provider.getCustomViewHolder(fullPath);
669+
670+
// place in window
671+
$("#editor-holder").append(_$currentCustomViewer);
735672

736-
// place in window
737-
$("#editor-holder").append(_$currentCustomViewer);
738-
739-
// add path, dimensions and file size to the view after loading image
740-
provider.render(fullPath);
741-
// make sure the file in display is still there when window gets focus.
742-
// close and warn if the file is gone.
743-
window.addEventListener("focus", _checkFileExists);
744-
_setCurrentlyViewedPath(fullPath);
745-
}
746-
}
747-
var file = FileSystem.getFileForPath(fullPath);
748-
file.exists(_doShow);
673+
// add path, dimensions and file size to the view after loading image
674+
provider.render(fullPath);
675+
676+
_setCurrentlyViewedPath(fullPath);
749677
}
750-
751678

752679
/**
753680
* Check whether the given file is currently open in a custom viewer.
@@ -789,6 +716,31 @@ define(function (require, exports, module) {
789716
return null;
790717
}
791718

719+
/**
720+
* Clears custom viewer for a file with a given path and displays
721+
* an alternate file or the no editor view.
722+
* If no param fullpath is passed an alternate file will be opened
723+
* regardless of the current value of _currentlyViewedPath.
724+
* If param fullpath is provided then only if fullpath matches
725+
* the currently viewed file an alternate file will be opened.
726+
* @param {?string} fullPath - file path of deleted file.
727+
*/
728+
function notifyPathDeleted(fullPath) {
729+
function openAlternateFile() {
730+
var fileToOpen = DocumentManager.getNextPrevFile(1);
731+
if (fileToOpen) {
732+
CommandManager.execute(Commands.FILE_OPEN, {fullPath: fileToOpen.fullPath});
733+
} else {
734+
_removeCustomViewer();
735+
_showNoEditor();
736+
_setCurrentlyViewedPath();
737+
}
738+
}
739+
if (!fullPath || _currentlyViewedPath === fullPath) {
740+
openAlternateFile();
741+
}
742+
}
743+
792744
/** Handles changes to DocumentManager.getCurrentDocument() */
793745
function _onCurrentDocumentChange() {
794746
var doc = DocumentManager.getCurrentDocument(),
@@ -1063,4 +1015,4 @@ define(function (require, exports, module) {
10631015
exports.notifyPathDeleted = notifyPathDeleted;
10641016
exports.closeCustomViewer = closeCustomViewer;
10651017
exports.showingCustomViewerForPath = showingCustomViewerForPath;
1066-
});
1018+
});

‎test/spec/DocumentCommandHandlers-test.js‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,8 +1068,14 @@ define(function (require, exports, module) {
10681068

10691069
describe("Opens image file and validates EditorManager APIs", function () {
10701070
it("should return null after opening an image", function () {
1071-
var path = testPath + "/couz.png";
1072-
CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }).done(function (result) {
1071+
var path = testPath + "/couz.png",
1072+
promise;
1073+
runs(function () {
1074+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: path });
1075+
waitsForDone(promise, Commands.FILE_OPEN);
1076+
});
1077+
1078+
runs(function () {
10731079
expect(EditorManager.getActiveEditor()).toEqual(null);
10741080
expect(EditorManager.getCurrentFullEditor()).toEqual(null);
10751081
expect(EditorManager.getFocusedEditor()).toEqual(null);
@@ -1082,12 +1088,10 @@ define(function (require, exports, module) {
10821088

10831089
describe("Open image file while a text file is open", function () {
10841090
it("should fire currentDocumentChange and activeEditorChange events", function () {
1085-
10861091
var promise,
10871092
docChangeListener = jasmine.createSpy(),
10881093
activeEditorChangeListener = jasmine.createSpy();
10891094

1090-
10911095
runs(function () {
10921096
_$(DocumentManager).on("currentDocumentChange", docChangeListener);
10931097
_$(EditorManager).on("activeEditorChange", activeEditorChangeListener);
@@ -1184,8 +1188,14 @@ define(function (require, exports, module) {
11841188

11851189
describe("Opens text file and validates EditorManager APIs", function () {
11861190
it("should return an editor after opening a text file", function () {
1187-
var path = testPath + "/test.js";
1188-
CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }).done(function (result) {
1191+
var path = testPath + "/test.js",
1192+
promise;
1193+
runs(function () {
1194+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: path });
1195+
waitsForDone(promise, Commands.FILE_OPEN);
1196+
});
1197+
1198+
runs(function () {
11891199
var e = EditorManager.getActiveEditor();
11901200
expect(e.document.file.fullPath).toBe(path);
11911201

0 commit comments

Comments
 (0)