19713 2012-10-26 05:46:47 +0000 Processing of event handler return value is ambiguous 2013-10-14 14:41:53 +0000 1 1 1 Unclassified WHATWG HTML unspecified Other other RESOLVED FIXED http://www.whatwg.org/specs/web-apps/current-work/#events P3 normal Unsorted 23489 1 contributor ian bzbarsky ian mike contributor oldest_to_newest 77128 0 contributor 2012-10-26 05:46:47 +0000 Specification: http://www.whatwg.org/specs/web-apps/current-work/multipage/webappapis.html Multipage: http://www.whatwg.org/C#events Complete: http://www.whatwg.org/c#events Comment: Processing of event handler return value is ambiguous Posted from: 173.48.30.113 by bzbarsky@mit.edu User agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/19.0 Firefox/19.0 77129 1 bzbarsky 2012-10-26 05:55:21 +0000 For example, this part: If the event type is mouseover If return value is a boolean with the value true, then cancel the event. This doesn't really define what "is a boolean" is. So it's not clear what should happen if the return value is the string "something". I would suggest relying on WebIDL here. In particular, make the IDL look somewhat like this: [TreatNonCallableAsNull] callback EventHandlerNonNull = boolean (Event event); typedef EventHandlerNonNull? EventHandler; [TreatNonCallableAsNull] callback BeforeUnloadEventHandlerNonNull = DOMString? (Event event); typedef BeforeUnloadEventHandlerNonNull? BeforeUnloadEventHandler; and define the onbeforeunload attribute as taking a BeforeUnloadEventHandler. Then the processing model can say: If the event type is mouseover If return value is true, then cancel the event. If the Event object E is a BeforeUnloadEvent object If return value is not null and the Event object E's returnValue attribute's value is the empty string, then set the returnValue attribute's value to return value. Otherwise If return value is false, then cancel the event. That should be completely unambiguous in weird cases like the onbeforeunload handler returning { toString: function() { return "hey, look at that" } }, which brings up a dialog in browsers but arguably shouldn't per current spec's "if the value is a string" bit. 77765 2 bzbarsky 2012-11-02 17:28:50 +0000 So the proposal in comment 1 doesn't work as-is because no return value would get treated as "false" for EventHandlerNonNull. We could do "boolean?" as the return type, and say that a non-null false is needed. That would still mean that returning "0" cancels the event; not sure whether that's desirable. 77766 3 bzbarsky 2012-11-02 17:33:41 +0000 But even if we decide that we can't have 0 preventing default, we should clearly define what "is a boolean" actually means here... in non-ECMAScript terms. :( 77770 4 bzbarsky 2012-11-02 19:22:39 +0000 Based on https://bugzilla.mozilla.org/show_bug.cgi?id=345521 sounds like we really don't want 0 to prevent default, unfortunately. 77780 5 ian 2012-11-02 21:39:23 +0000 "Is a JavaScript boolean with the value false"? 78809 6 bzbarsky 2012-11-26 04:53:58 +0000 The return value is a WebIDL value... It may be that the best we can do here is keep using IDL "any" and just make sure that this is "boolean" in the WebIDL sense, and rely on http://dev.w3.org/2006/webapi/WebIDL/#es-any to do the right thing for us. I still think beforeunload should have DOMString? as return value, though. 82663 7 ian 2013-02-06 23:38:10 +0000 bz: Using special WebIDL callbacks for beforeunload doesn't work because BeforeUnloadEvent objects can be fired for any event name, not just 'beforeunload'. But I've tried to fix it... let me know (and reopen) if it's still problematic. 82664 8 contributor 2013-02-06 23:38:21 +0000 Checked in as WHATWG revision r7702. Check-in comment: Be more explicit about what this means. http://html5.org/tools/web-apps-tracker?from=7701&to=7702 82761 9 bzbarsky 2013-02-08 16:58:58 +0000 > because BeforeUnloadEvent objects can be fired for any event name, not just > 'beforeunload'. I don't follow this. Can you explain? As far as I can tell the spec as currently written doesn't match implementations very well (not that interop for beforeunload is all that great). 85125 10 ian 2013-03-28 20:28:34 +0000 What do you think the spec _should_ say? The options I see are: 1. only have this magic behaviour for the beforeunload events fired as part of the "prompt to unload a document" algorithm, 2. have this magic behaviour for all BeforeUnloadEvent events 3. have this magic behaviour for all BeforeUnloadEvent events named 'beforeunload' Is there a fourth option I'm missing? Option 1 seems bad because it means the event dispatch mechanism has a dependency on the otherwise unrelated unload logic. Option 3, as far as I can tell, needs a few more cycles to do something less useful than option 2. That leaves option 2, which is what the spec says. I agree that interop here isn't so hot, which is why I haven't really attempted to precisely match any UA, but obviously if there's a compat need, or if there's more interop than I realise, or if there's a fourth better option, or something, then we should change this. 85135 11 bzbarsky 2013-03-29 00:09:38 +0000 > What do you think the spec _should_ say? What Gecko implements at this point is as follows: 1) onbeforeunload has the type BeforeUnloadEventHandler on window/body/frameset, defined like so: callback BeforeUnloadEventHandlerNonNull = DOMString? (Event event); typedef BeforeUnloadEventHandlerNonNull? BeforeUnloadEventHandler; 2) When invoking a BeforeUnloadEventHandlerNonNull, if the return value is not null call preventDefault() on the event object. Then if the event is a BeforeUnloadEvent and its returnValue is empty, set its returnValue to the return value of the handler. In particular, if the handler returns the number 42, or { toString: function() { return "hey there"; } } , we will show a prompt whereas current spec will not, as far as I can tell. Furthermore, returning non-null from the handler will call preventDefault on the event, whereas the spec doesn't do that. Those seem to be the main web-page detectable differences between what the spec says and Gecko does. 87234 12 ian 2013-05-04 00:31:07 +0000 So that would be: 4. have 'beforeunload' events' return values be coerced to DOMString or null (unlike other events which aren't coerced at all), cancel if it's null, and if it's also a BeforeUnloadEvent event, additionally set the event object's returnValue based on the returned string if it's not null. Is that right? 87235 13 bzbarsky 2013-05-04 01:17:31 +0000 That's what Gecko implements right now, I believe, yes. 88909 14 ian 2013-06-07 20:58:14 +0000 Thanks for your patience with me on this, as always. 88910 15 contributor 2013-06-07 21:01:16 +0000 Checked in as WHATWG revision r7933. Check-in comment: Bring onbeforeunload handling closer to compatibility with the Web http://html5.org/tools/web-apps-tracker?from=7932&to=7933