[webkit-reviews] review denied: [Bug 52855] WebKit2: Implement showModalDialog : [Attachment 79680] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has denied Darin Adler <darin at apple.com>'s request for review:
Bug 52855: WebKit2: Implement showModalDialog
https://bugs.webkit.org/show_bug.cgi?id=52855

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

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=79680&action=review

Looks good but missing a piece.

> Source/WebKit2/ChangeLog:47
> +	   * WebProcess/WebPage/WebPage.h: Defined them canRunModal function

Typo: them

> Source/WebKit2/ChangeLog:48
> +	   and declared these runModal function.

Another typo(?): these

> Source/WebKit2/UIProcess/API/C/WKPage.h:173
> -    const void *							  
clientInfo;
> +    const void*							  
clientInfo;

I believe “void *” is the CF API convention.

> Tools/ChangeLog:19
> +	   * WebKitTestRunner/TestController.cpp:
> +	   (WTR::TestController::runModal): Added. Calls through to the
> +	   platform-specific version of runModal.
> +	   (WTR::TestController::createOtherPage): Changed to be a private
> +	   static member function so it can refer to runModal, which is
> +	   a private static member function.
> +	   (WTR::TestController::initialize): Pass 0 for the runModal
> +	   function since we don't need to run the main window modal.
> +	   I suspect this is wrong and will need to change.

These changes aren’t included in the patch.