[Webkit-unassigned] [Bug 188696] beforeunload interoperability issues with a throwing return

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188696

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com
 Attachment #347442|review?, commit-queue?      |review-, commit-queue-
              Flags|                            |

--- Comment #15 from Darin Adler <darin at apple.com> ---
Comment on attachment 347442
  --> https://bugs.webkit.org/attachment.cgi?id=347442
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347442&action=review

Thanks for contributing this patch. Seems like a good change to make! Almost there, but needs a little work. Need to add a test, and there’s a mistake in the patch that a good test would find.

> Source/WebCore/ChangeLog:8
> +        No new tests .

Normally we’d say *why* there are no new tests. If there is a change in behavior, the project requires we add a test to demonstrate that the patch works. Can we construct a test? I think we should be able to do this by passing an object that shows an exception in its valueOf function. We should look at our existing beforeunload tests to see if one can be adapted for this purpose. Ideally another test will cover the case where the valueOf function does not return an exception and ensure that it is called exactly once. That would have caught the mistake in the patch that I mention below.

> Source/WebCore/bindings/js/JSEventListener.cpp:194
> +                    reportException(exec, exception);

We might need something like the call to uncaughtExceptionInEventHandler above. Not sure if we need it. Might require a little research.

> Source/WebCore/bindings/js/JSEventListener.cpp:196
> +                    handleBeforeUnloadEventReturnValue(downcast<BeforeUnloadEvent>(event), convert<IDLNullable<IDLDOMString>>(*exec, retval));

This needs to pass returnedString, and not compute it a second time.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180820/3687bf34/attachment.html>