[webkit-reviews] review denied: [Bug 56397] Suppress modal JavaScript/HTML dialogs during unload events : [Attachment 86035] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Sreeram Ramachandran
<sreeram at google.com>'s request for review:
Bug 56397: Suppress modal JavaScript/HTML dialogs during unload events
https://bugs.webkit.org/show_bug.cgi?id=56397

Attachment 86035: Patch
https://bugs.webkit.org/attachment.cgi?id=86035&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=86035&action=review

> LayoutTests/fast/loader/unload-alert.html:9
> +  // Once dialogs are disabled on all platforms, change these to "FAIL" so
that
> +  // if the dialogs appear, it's obvious that the test failed.
> +  alert("PASS");
> +  confirm("PASS");
> +  prompt("PASS", "PASS");

This test doesn't make sense on chromium, does it?  If any of PASS appear on
chromium platform, it actually failed, right?  I don't think it's clear at all
from the test output or from the comment here.

> Source/WebCore/page/Chrome.cpp:62
> +static inline bool canRunModalDuringUnload()
> +{
> +#if PLATFORM(CHROMIUM)
> +    return false;
> +#else
> +    return true;
> +#endif
> +}

I don't think this is the right way to do it if we want to make decisions per
port.  We should add a new callback to WebView or some other class instead.