diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index c165390e636..73197bf9e54 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -36,6 +36,7 @@ define(function (require, exports, module) { DocumentManager = require("document/DocumentManager"), EditorManager = require("editor/EditorManager"), FileSystem = require("filesystem/FileSystem"), + FileSystemError = require("filesystem/FileSystemError"), FileUtils = require("file/FileUtils"), FileViewController = require("project/FileViewController"), InMemoryFile = require("document/InMemoryFile"), @@ -178,7 +179,43 @@ define(function (require, exports, module) { */ function doOpen(fullPath, silent) { var result = new $.Deferred(); - + + // workaround for https://github.com/adobe/brackets/issues/6001 + // TODO should be removed once bug is closed. + // if we are already displaying a file do nothing but resolve immediately. + // this fixes timing issues in test cases. + if (EditorManager.getCurrentlyViewedPath() === fullPath) { + result.resolve(DocumentManager.getCurrentDocument()); + return result.promise(); + } + + function _cleanup(fullFilePath) { + if (!fullFilePath || EditorManager.showingCustomViewerForPath(fullFilePath)) { + // We get here only after the user renames a file that makes it no longer belong to a + // custom viewer but the file is still showing in the current custom viewer. This only + // occurs on Mac since opening a non-text file always fails on Mac and triggers an error + // message that in turn calls _cleanup() after the user clicks OK in the message box. + // So we need to explicitly close the currently viewing image file whose filename is + // no longer valid. Calling notifyPathDeleted will close the image vieer and then select + // the previously opened text file or show no-editor if none exists. + EditorManager.notifyPathDeleted(fullFilePath); + } else { + // For performance, we do lazy checking of file existence, so it may be in working set + DocumentManager.removeFromWorkingSet(FileSystem.getFileForPath(fullFilePath)); + EditorManager.focusEditor(); + } + result.reject(); + } + function _showErrorAndCleanUp(fileError, fullFilePath) { + if (silent) { + _cleanup(fullFilePath); + } else { + FileUtils.showFileOpenError(fileError, fullFilePath).done(function () { + _cleanup(fullFilePath); + }); + } + } + if (!fullPath) { console.error("doOpen() called without fullPath"); result.reject(); @@ -190,8 +227,17 @@ define(function (require, exports, module) { var viewProvider = EditorManager.getCustomViewerForPath(fullPath); if (viewProvider) { - EditorManager.showCustomViewer(viewProvider, fullPath); - result.resolve(); + var file = FileSystem.getFileForPath(fullPath); + file.exists(function (fileError, fileExists) { + if (fileExists) { + EditorManager.showCustomViewer(viewProvider, fullPath); + result.resolve(); + } else { + fileError = fileError || FileSystemError.NOT_FOUND; + _showErrorAndCleanUp(fileError); + } + }); + } else { // Load the file if it was never open before, and then switch to it in the UI DocumentManager.getDocumentForPath(fullPath) @@ -200,29 +246,7 @@ define(function (require, exports, module) { result.resolve(doc); }) .fail(function (fileError) { - function _cleanup() { - if (EditorManager.showingCustomViewerForPath(fullPath)) { - // We get here only after the user renames a file that makes it no longer belong to a - // custom viewer but the file is still showing in the current custom viewer. This only - // occurs on Mac since opening a non-text file always fails on Mac and triggers an error - // message that in turn calls _cleanup() after the user clicks OK in the message box. - // So we need to explicitly close the currently viewing image file whose filename is - // no longer valid. Calling notifyPathDeleted will close the image vieer and then select - // the previously opened text file or show no-editor if none exists. - EditorManager.notifyPathDeleted(fullPath); - } else { - // For performance, we do lazy checking of file existence, so it may be in working set - DocumentManager.removeFromWorkingSet(FileSystem.getFileForPath(fullPath)); - EditorManager.focusEditor(); - } - result.reject(); - } - - if (silent) { - _cleanup(); - } else { - FileUtils.showFileOpenError(fileError, fullPath).done(_cleanup); - } + _showErrorAndCleanUp(fileError, fullPath); }); } } diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index a36b17f33d2..7a1e778e0b4 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -60,10 +60,7 @@ define(function (require, exports, module) { InlineTextEditor = require("editor/InlineTextEditor").InlineTextEditor, ImageViewer = require("editor/ImageViewer"), Strings = require("strings"), - LanguageManager = require("language/LanguageManager"), - FileSystem = require("filesystem/FileSystem"), - FileSystemError = require("filesystem/FileSystemError"), - FileUtils = require("file/FileUtils"); + LanguageManager = require("language/LanguageManager"); /** @type {jQueryObject} DOM node that contains all editors (visible and hidden alike) */ var _editorHolder = null; @@ -81,8 +78,6 @@ define(function (require, exports, module) { var _$currentCustomViewer = null; /** @type {?Object} view provider */ var _currentViewProvider = null; - /** Helper function defined as var to satisfy JSLint order constraints */ - var _checkFileExists; /** * Currently focused Editor (full-size, inline, or otherwise) @@ -630,7 +625,6 @@ define(function (require, exports, module) { /** Remove existing custom view if present */ function _removeCustomViewer() { - window.removeEventListener("focus", _checkFileExists); $(exports).triggerHandler("removeCustomViewer"); if (_$currentCustomViewer) { _$currentCustomViewer.remove(); @@ -638,62 +632,6 @@ define(function (require, exports, module) { _$currentCustomViewer = null; _currentViewProvider = null; } - - /** - * Clears custom viewer for a file with a given path and displays - * an alternate file or the no editor view. - * If no param fullpath is passed an alternate file will be opened - * regardless of the current value of _currentlyViewedPath. - * If param fullpath is provided then only if fullpath matches - * the currently viewed file an alternate file will be opened. - * @param {?string} fullPath - file path of deleted file. - */ - function notifyPathDeleted(fullPath) { - function openAlternateFile() { - var fileToOpen = DocumentManager.getNextPrevFile(1); - if (fileToOpen) { - CommandManager.execute(Commands.FILE_OPEN, {fullPath: fileToOpen.fullPath}); - } else { - _removeCustomViewer(); - _showNoEditor(); - _setCurrentlyViewedPath(); - } - } - if (!fullPath || _currentlyViewedPath === fullPath) { - openAlternateFile(); - } - } - - /* - * show a generic error or File Not Found in modal error dialog - */ - function _showErrorAndNotify(err, fullPath) { - var errorToShow = err || FileSystemError.NOT_FOUND; - FileUtils.showFileOpenError(errorToShow, fullPath).done( - function () { - notifyPathDeleted(); - } - ); - } - - /* - * callback function passed to file.exists. If file in view does - * not exist the current view will be replaced. - */ - function _removeViewIfFileDeleted(err, fileExists) { - if (!fileExists) { - notifyPathDeleted(); - } - } - - /** - * Makes sure that the file in view is present in the file system - * Close and warn if file is gone. - */ - _checkFileExists = function () { - var file = FileSystem.getFileForPath(getCurrentlyViewedPath()); - file.exists(_removeViewIfFileDeleted); - }; /** * Closes the customViewer currently displayed, shows the NoEditor view @@ -704,50 +642,39 @@ define(function (require, exports, module) { _setCurrentlyViewedPath(); _showNoEditor(); } - + /** * Append custom view to editor-holder * @param {!Object} provider custom view provider * @param {!string} fullPath path to the file displayed in the custom view */ function showCustomViewer(provider, fullPath) { - function _doShow(err, fileExists) { - if (!fileExists) { - _showErrorAndNotify(err, fullPath); - } else { - // Don't show the same custom view again if file path - // and view provider are still the same. - if (_currentlyViewedPath === fullPath && - _currentViewProvider === provider) { - return; - } - - // Clean up currently viewing document or custom viewer - DocumentManager._clearCurrentDocument(); - _removeCustomViewer(); - - // Hide the not-editor or reset current editor - $("#not-editor").css("display", "none"); - _nullifyEditor(); + // Don't show the same custom view again if file path + // and view provider are still the same. + if (_currentlyViewedPath === fullPath && + _currentViewProvider === provider) { + return; + } - _currentViewProvider = provider; - _$currentCustomViewer = provider.getCustomViewHolder(fullPath); + // Clean up currently viewing document or custom viewer + DocumentManager._clearCurrentDocument(); + _removeCustomViewer(); + + // Hide the not-editor or reset current editor + $("#not-editor").css("display", "none"); + _nullifyEditor(); + + _currentViewProvider = provider; + _$currentCustomViewer = provider.getCustomViewHolder(fullPath); + + // place in window + $("#editor-holder").append(_$currentCustomViewer); - // place in window - $("#editor-holder").append(_$currentCustomViewer); - - // add path, dimensions and file size to the view after loading image - provider.render(fullPath); - // make sure the file in display is still there when window gets focus. - // close and warn if the file is gone. - window.addEventListener("focus", _checkFileExists); - _setCurrentlyViewedPath(fullPath); - } - } - var file = FileSystem.getFileForPath(fullPath); - file.exists(_doShow); + // add path, dimensions and file size to the view after loading image + provider.render(fullPath); + + _setCurrentlyViewedPath(fullPath); } - /** * Check whether the given file is currently open in a custom viewer. @@ -789,6 +716,31 @@ define(function (require, exports, module) { return null; } + /** + * Clears custom viewer for a file with a given path and displays + * an alternate file or the no editor view. + * If no param fullpath is passed an alternate file will be opened + * regardless of the current value of _currentlyViewedPath. + * If param fullpath is provided then only if fullpath matches + * the currently viewed file an alternate file will be opened. + * @param {?string} fullPath - file path of deleted file. + */ + function notifyPathDeleted(fullPath) { + function openAlternateFile() { + var fileToOpen = DocumentManager.getNextPrevFile(1); + if (fileToOpen) { + CommandManager.execute(Commands.FILE_OPEN, {fullPath: fileToOpen.fullPath}); + } else { + _removeCustomViewer(); + _showNoEditor(); + _setCurrentlyViewedPath(); + } + } + if (!fullPath || _currentlyViewedPath === fullPath) { + openAlternateFile(); + } + } + /** Handles changes to DocumentManager.getCurrentDocument() */ function _onCurrentDocumentChange() { var doc = DocumentManager.getCurrentDocument(), @@ -1063,4 +1015,4 @@ define(function (require, exports, module) { exports.notifyPathDeleted = notifyPathDeleted; exports.closeCustomViewer = closeCustomViewer; exports.showingCustomViewerForPath = showingCustomViewerForPath; -}); +}); \ No newline at end of file diff --git a/test/spec/DocumentCommandHandlers-test.js b/test/spec/DocumentCommandHandlers-test.js index b2a238f5953..9376446d9e1 100644 --- a/test/spec/DocumentCommandHandlers-test.js +++ b/test/spec/DocumentCommandHandlers-test.js @@ -1068,8 +1068,14 @@ define(function (require, exports, module) { describe("Opens image file and validates EditorManager APIs", function () { it("should return null after opening an image", function () { - var path = testPath + "/couz.png"; - CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }).done(function (result) { + var path = testPath + "/couz.png", + promise; + runs(function () { + promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }); + waitsForDone(promise, Commands.FILE_OPEN); + }); + + runs(function () { expect(EditorManager.getActiveEditor()).toEqual(null); expect(EditorManager.getCurrentFullEditor()).toEqual(null); expect(EditorManager.getFocusedEditor()).toEqual(null); @@ -1082,12 +1088,10 @@ define(function (require, exports, module) { describe("Open image file while a text file is open", function () { it("should fire currentDocumentChange and activeEditorChange events", function () { - var promise, docChangeListener = jasmine.createSpy(), activeEditorChangeListener = jasmine.createSpy(); - runs(function () { _$(DocumentManager).on("currentDocumentChange", docChangeListener); _$(EditorManager).on("activeEditorChange", activeEditorChangeListener); @@ -1184,8 +1188,14 @@ define(function (require, exports, module) { describe("Opens text file and validates EditorManager APIs", function () { it("should return an editor after opening a text file", function () { - var path = testPath + "/test.js"; - CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }).done(function (result) { + var path = testPath + "/test.js", + promise; + runs(function () { + promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }); + waitsForDone(promise, Commands.FILE_OPEN); + }); + + runs(function () { var e = EditorManager.getActiveEditor(); expect(e.document.file.fullPath).toBe(path);