[webkit-reviews] review denied: [Bug 56397] Suppress modal JavaScript/HTML dialogs during unload events : [Attachment 88926] 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 88926: Patch
https://bugs.webkit.org/attachment.cgi?id=88926&action=review

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

Please separate changes to the existing tests because there's no reason they
should be in the same patch.

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

I don't think this is the right way to do it.  Since we already have
willRunModalDialogDuringPageDismissal on chrome client, we should rename it to
shouldRunModalDialogDuringPageDismissal and have it return a boolean value.  I
know that we want all ports to eventually disallow modal dialogs but that's not
the case at least for now so it seems more natural to have chrome client decide
it.