Browse Source

Adjust to the new behaviors of beforeunload handler

Cheng Zhao 9 years ago
parent
commit
fa0ce7ad5f

+ 1 - 1
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

+ 5 - 5
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;
 };
 ```

+ 2 - 9
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) {

+ 0 - 13
spec/fixtures/api/close-beforeunload-string.html

@@ -1,13 +0,0 @@
-<html>
-<body>
-<script type="text/javascript" charset="utf-8">
-  window.onbeforeunload = function() {
-    setTimeout(function() {
-      require('electron').remote.getCurrentWindow().emit('onbeforeunload');
-    }, 0);
-    return 'string';
-  }
-  window.close();
-</script>
-</body>
-</html>

+ 0 - 1
spec/fixtures/api/close-beforeunload-true.html → spec/fixtures/api/close-beforeunload-undefined.html

@@ -5,7 +5,6 @@
     setTimeout(function() {
       require('electron').remote.getCurrentWindow().emit('onbeforeunload');
     }, 0);
-    return true;
   }
   window.close();
 </script>