From 78e07b1ca91daef3e0a78436d370501dcd816f2d Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Wed, 5 Jun 2013 11:13:53 -0700 Subject: [PATCH 1/3] update usage of new Dialogs API: done() does not chain, command handlers should return a promise. --- src/LiveDevelopment/LiveDevelopment.js | 2 +- src/preferences/PreferencesDialogs.js | 25 +++++++++++++------------ src/project/ProjectManager.js | 2 +- src/utils/ShellAPI.js | 2 +- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/LiveDevelopment/LiveDevelopment.js b/src/LiveDevelopment/LiveDevelopment.js index 8b9794d6d61..67b4d4d647b 100644 --- a/src/LiveDevelopment/LiveDevelopment.js +++ b/src/LiveDevelopment/LiveDevelopment.js @@ -572,7 +572,7 @@ define(function LiveDevelopment(require, exports, module) { function _onDetached(event, res) { // If there already is a reason for closing the session, do not overwrite it - if (!_closeReason) { + if (!_closeReason && res && res.reason) { // Get the explanation from res.reason, e.g. "replaced_with_devtools", "target_closed", "canceled_by_user" // Examples taken from https://chromiumcodereview.appspot.com/10947037/patch/12001/13004 // However, the link refers to the Chrome Extension API, it may not apply 100% to the Inspector API diff --git a/src/preferences/PreferencesDialogs.js b/src/preferences/PreferencesDialogs.js index 8b8a50b9e66..07bf1c1d82a 100644 --- a/src/preferences/PreferencesDialogs.js +++ b/src/preferences/PreferencesDialogs.js @@ -85,19 +85,20 @@ define(function (require, exports, module) { $baseUrlControl, dialog; - dialog = Dialogs.showModalDialogUsingTemplate(Mustache.render(SettingsDialogTemplate, Strings)) - .done(function (id) { - if (id === Dialogs.DIALOG_BTN_OK) { - var baseUrlValue = $baseUrlControl.val(); - var result = _validateBaseUrl(baseUrlValue); - if (result === "") { - ProjectManager.setBaseUrl(baseUrlValue); - } else { - // Re-invoke dialog with result (error message) - showProjectPreferencesDialog(baseUrlValue, result); - } + dialog = Dialogs.showModalDialogUsingTemplate(Mustache.render(SettingsDialogTemplate, Strings)); + + dialog.done(function (id) { + if (id === Dialogs.DIALOG_BTN_OK) { + var baseUrlValue = $baseUrlControl.val(); + var result = _validateBaseUrl(baseUrlValue); + if (result === "") { + ProjectManager.setBaseUrl(baseUrlValue); + } else { + // Re-invoke dialog with result (error message) + showProjectPreferencesDialog(baseUrlValue, result); } - }); + } + }); // Populate project settings $dlg = $(".project-settings-dialog.instance"); diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index c1f171ab6f1..6f6833239d2 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -1107,7 +1107,7 @@ define(function (require, exports, module) { * @return {Dialog} */ function _projectSettings() { - return PreferencesDialogs.showProjectPreferencesDialog(getBaseUrl()); + return PreferencesDialogs.showProjectPreferencesDialog(getBaseUrl()).getPromise(); } /** diff --git a/src/utils/ShellAPI.js b/src/utils/ShellAPI.js index e10ec40ec94..f19ba03b9dc 100644 --- a/src/utils/ShellAPI.js +++ b/src/utils/ShellAPI.js @@ -67,7 +67,7 @@ define(function (require, exports, module) { "at a breakpoint. If you are NOT at a breakpoint, please " + "file a bug and mention this comment. Stack depth = " + stackDepth + "."); } - return (promise && promise.state() === "rejected") ? false : true; + return (promise && (typeof promise.state === "function") && promise.state() === "rejected") ? false : true; } AppInit.appReady(function () { From 60165476b7c7e3116e2515126b905c102f27d2ca Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Thu, 6 Jun 2013 11:33:59 -0700 Subject: [PATCH 2/3] ShellAPI should expect commands to return undefined or a promise --- src/utils/ShellAPI.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/ShellAPI.js b/src/utils/ShellAPI.js index f19ba03b9dc..e10ec40ec94 100644 --- a/src/utils/ShellAPI.js +++ b/src/utils/ShellAPI.js @@ -67,7 +67,7 @@ define(function (require, exports, module) { "at a breakpoint. If you are NOT at a breakpoint, please " + "file a bug and mention this comment. Stack depth = " + stackDepth + "."); } - return (promise && (typeof promise.state === "function") && promise.state() === "rejected") ? false : true; + return (promise && promise.state() === "rejected") ? false : true; } AppInit.appReady(function () { From e0a767c2b975d1a5f6dfe32a6af0b2eb1b26cd17 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Thu, 6 Jun 2013 14:15:25 -0700 Subject: [PATCH 3/3] code review comments --- src/preferences/PreferencesDialogs.js | 2 +- src/project/ProjectManager.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/preferences/PreferencesDialogs.js b/src/preferences/PreferencesDialogs.js index 07bf1c1d82a..d00305a201c 100644 --- a/src/preferences/PreferencesDialogs.js +++ b/src/preferences/PreferencesDialogs.js @@ -101,7 +101,7 @@ define(function (require, exports, module) { }); // Populate project settings - $dlg = $(".project-settings-dialog.instance"); + $dlg = dialog.getElement(); // Title $title = $dlg.find(".dialog-title"); diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index 6f6833239d2..9eaf427dc0c 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -1104,7 +1104,7 @@ define(function (require, exports, module) { /** * Invoke project settings dialog. - * @return {Dialog} + * @return {$.Promise} */ function _projectSettings() { return PreferencesDialogs.showProjectPreferencesDialog(getBaseUrl()).getPromise();