From fa0ce7ad5ff63e016a75047d7c0271a3baec5398 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 May 2016 13:28:16 +0900 Subject: [PATCH] Adjust to the new behaviors of beforeunload handler --- atom/browser/atom_javascript_dialog_manager.cc | 2 +- docs/api/browser-window.md | 10 +++++----- spec/api-browser-window-spec.js | 11 ++--------- spec/fixtures/api/close-beforeunload-string.html | 13 ------------- ...-true.html => close-beforeunload-undefined.html} | 1 - 5 files changed, 8 insertions(+), 29 deletions(-) delete mode 100644 spec/fixtures/api/close-beforeunload-string.html rename spec/fixtures/api/{close-beforeunload-true.html => close-beforeunload-undefined.html} (93%) diff --git a/atom/browser/atom_javascript_dialog_manager.cc b/atom/browser/atom_javascript_dialog_manager.cc index 07f3ea69bb7e..eaf311ba1cdb 100644 --- a/atom/browser/atom_javascript_dialog_manager.cc +++ b/atom/browser/atom_javascript_dialog_manager.cc @@ -26,7 +26,7 @@ void AtomJavaScriptDialogManager::RunBeforeUnloadDialog( bool is_reload, const DialogClosedCallback& callback) { // FIXME(zcbenz): the |message_text| is removed, figure out what should we do. - callback.Run(true, base::ASCIIToUTF16("FIXME")); + callback.Run(false, base::ASCIIToUTF16("This should not be displayed")); } } // namespace atom diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 0bd38dec560e..c2bf46d46366 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -217,17 +217,17 @@ will cancel the close. Usually you would want to use the `beforeunload` handler to decide whether the window should be closed, which will also be called when the window is -reloaded. In Electron, returning an empty string or `false` would cancel the +reloaded. In Electron, returning any value other than `undefined` would cancel the close. For example: ```javascript window.onbeforeunload = (e) => { console.log('I do not want to be closed'); - // Unlike usual browsers, in which a string should be returned and the user is - // prompted to confirm the page unload, Electron gives developers more options. - // Returning empty string or false would prevent the unloading now. - // You can also use the dialog API to let the user confirm closing the application. + // Unlike usual browsers that a message box will be prompted to users, returning + // a non-void value will silently cancel the close. + // It is recommended to use the dialog API to let the user confirm closing the + // application. e.returnValue = false; }; ``` diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index ac85f49068d4..3b1ccabaae59 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -476,18 +476,11 @@ describe('browser-window module', function () { }) describe('beforeunload handler', function () { - it('returning true would not prevent close', function (done) { + it('returning undefined would not prevent close', function (done) { w.on('closed', function () { done() }) - w.loadURL('file://' + path.join(fixtures, 'api', 'close-beforeunload-true.html')) - }) - - it('returning non-empty string would not prevent close', function (done) { - w.on('closed', function () { - done() - }) - w.loadURL('file://' + path.join(fixtures, 'api', 'close-beforeunload-string.html')) + w.loadURL('file://' + path.join(fixtures, 'api', 'close-beforeunload-undefined.html')) }) it('returning false would prevent close', function (done) { diff --git a/spec/fixtures/api/close-beforeunload-string.html b/spec/fixtures/api/close-beforeunload-string.html deleted file mode 100644 index c8c6c6beacdd..000000000000 --- a/spec/fixtures/api/close-beforeunload-string.html +++ /dev/null @@ -1,13 +0,0 @@ - - - - - diff --git a/spec/fixtures/api/close-beforeunload-true.html b/spec/fixtures/api/close-beforeunload-undefined.html similarity index 93% rename from spec/fixtures/api/close-beforeunload-true.html rename to spec/fixtures/api/close-beforeunload-undefined.html index 6de0c6cd3989..b4e4178f6a1d 100644 --- a/spec/fixtures/api/close-beforeunload-true.html +++ b/spec/fixtures/api/close-beforeunload-undefined.html @@ -5,7 +5,6 @@ setTimeout(function() { require('electron').remote.getCurrentWindow().emit('onbeforeunload'); }, 0); - return true; } window.close();