onbeforeunload return value processing is not per-spec |
|||||||
Issue descriptionChrome Version: Version 58.0.3012.0 (Official Build) canary (64-bit) OS: Windows 10 What steps will reproduce the problem? Run the tests at https://w3c-test.org/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling.html. There are two issues: 1. The beforeunload dialog shows even for iframes, which is a bit surprising to me. That is not this bug, so just click "Leave" multiple times. 2. The failing tests. The spec for this is at https://html.spec.whatwg.org/multipage/webappapis.html#the-event-handler-processing-algorithm. In particular, the return value processing is supposed to convert whatever value is given to a Web IDL DOMString?, and then react whenever it is non-null. That means inputs like "", true, 0 are supposed to cancel the event, whereas it appears in Chrome they do not.
,
Mar 28 2017
They pass in Safari Tech Preview and in Firefox
,
Mar 28 2017
,
Apr 5 2017
,
Apr 24 2017
http://stackoverflow.com/questions/10680544/beforeunload-chrome-issue might be relevant. It looks like Document::DispatchBeforeUnloadEvent just consults DefaultPrevented and ReturnValue.IsNull but the IDL has DOMString and not DOMString?
,
Aug 22 2017
Bulk disowning per sshruthi's email about bug triage best practices.
,
Oct 11 2017
,
Oct 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/530a82542c23ba43cd3bce7c325b78820bc636d5 commit 530a82542c23ba43cd3bce7c325b78820bc636d5 Author: Rakina Zata Amni <rakina@chromium.org> Date: Fri Oct 20 10:34:41 2017 Don't cancel Events with type beforeunload If we return the boolean false in a beforeunload eventlistener, the unload event will get canceled. This is not per-spec, which is in https://html.spec.whatwg.org/multipage/webappapis.html#the-event-handler-processing-algorithm This is because the return value of the event listener is not converted to DOMString, because OnBeforeUnloadEventHandler is not currently implemented. See: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/WindowEventHandlers.idl?q=beforeunloadeventhandler&dr=C&l=39 This is a temporary fix until OnBeforeUnloadEventHandler is implemented, making sure that Events with the type beforeunload won't get canceled (in accordance to the spec). Bug: 692203 Change-Id: Ic31e8b90fb2edad173e99ac6a7d9059162a53278 Reviewed-on: https://chromium-review.googlesource.com/727863 Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Takayoshi Kochi <kochi@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/heads/master@{#510398} [delete] https://crrev.com/2cd618735fb87636b100642cacbe8705b9ab3199/third_party/WebKit/LayoutTests/external/wpt/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt [modify] https://crrev.com/530a82542c23ba43cd3bce7c325b78820bc636d5/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp
,
Oct 20 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dcheng@chromium.org
, Mar 27 2017