Closed Bug 1315454 Opened 8 years ago Closed 8 years ago

onbeforeunload causes alert even by returning false.

Categories

(Core :: DOM: Events, defect)

49 Branch
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: christoph, Unassigned)

Details

(Keywords: testcase)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20161025170400

Steps to reproduce:

- Create a page with a checkbox:

    <input type="checkbox" id="checkbox" />

 that sets onbeforeunload:

    jQuery(window).on({
        beforeunload: function() { return window.checkbox.checked; }
    });

- Interact with the page (beforeunload otherwise tends to be bypassed)

- Leave page while checkbox is unchecked.


Actual results:

Firefox shows an alert and confirmation dialog.


Expected results:

Since the event handler returned false, no alert should have been shown.

A workaround for this bug is to replace all such return statements with `return returnValue || undefined;`, because returning `undefined` will cause the expected behavior.
Could you provide a testcase (on https://jsfiddle.net/ e.g.), please.
Flags: needinfo?(christoph)
Keywords: testcase-wanted
Sure, here: https://jsfiddle.net/2rd8qdj6/7/

When the first checkbox is not checked, clicking the Reload button shouldn't trigger a warning as the event handler returns false.

Checking the second box will apply the workaround by replacing false with undefined, after which the first checkbox will have the desired effect.
Flags: needinfo?(christoph)
Component: Untriaged → DOM: Events
(In reply to christoph from comment #2)
> Sure, here: https://jsfiddle.net/2rd8qdj6/7/
> 
> When the first checkbox is not checked, clicking the Reload button shouldn't
> trigger a warning as the event handler returns false.
> 
> Checking the second box will apply the workaround by replacing false with
> undefined, after which the first checkbox will have the desired effect.

Got the same behaviour in Opera and Chrome.
Just did some testing in Chrome and Firefox:

1. setting to an empty function has no effect.
2. returning false, shows the prompt. 
3. returning true, shows the prompt.
4. explicitly returning undefined, does not show the prompt. 

Need to check what the HTML spec says here.
Need info myself to check HTML spec as to the correct behavior - but I think this bug might be invalid.
Flags: needinfo?(mcaceres)
Ok, so onbeforeunload is an oddball event, actually coerces the return value to a string. Here is the WebIDL definition for it:

```WebIDL
[TreatNonCallableAsNull]
callback OnBeforeUnloadEventHandlerNonNull = DOMString? (Event event);
``` 

Then, HTML spec then says:

"If the returnValue attribute of the event object is not the empty string, or if the event was canceled, then the user agent should ask the user to confirm that they wish to unload the document."

So, returning undefined works because nothing is returned... but if you return `false` is gets converted to the string "false" - same with `true`.

The correct usage is thus:

```
window.onbeforeunload = function() {
  return (window.checkbox1.checked) : "Stop!" : ""; // or undefined
}
```

Hope that helps!
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
Flags: needinfo?(mcaceres)
Resolution: --- → INVALID
Thanks Marcos for checking the spec. :)
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #7)
> Thanks Marcos for checking the spec. :)

No problem.

@christoph: According to the spec, you should also be able to do the following. Unfortunately, there is a bug in Chrome that doesn't allow the following to work (although it works in Firefox):

```
window.onbeforeunload = function(ev) {
  ev.preventDefault();
}
```

https://jsfiddle.net/2rd8qdj6/26/

So the safest way seem to be either return a truthy string (prevent) and an empty string or undefined (allow).
Followed up a bit more over in the HTML repo, you can also return "null":

```
window.onbeforeunload = function() {
  return (window.checkbox1.checked) : "Stop!" : ""; // empty string, undefined, and null all should work. 
}
```

It's basically defined here:
https://html.spec.whatwg.org/#the-event-handler-processing-algorithm
You need to log in before you can comment on or make changes to this bug.