[webkit-reviews] review denied: [Bug 16562] [gtk] Implement WebPolicyDelegate methods : [Attachment 18394] Applied comments and more

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has denied Pierre-Luc Beaudoin
<pierre-luc.beaudoin at collabora.co.uk>'s request for review:
Bug 16562: [gtk] Implement WebPolicyDelegate methods
http://bugs.webkit.org/show_bug.cgi?id=16562

Attachment 18394: Applied comments and more
http://bugs.webkit.org/attachment.cgi?id=18394&action=edit

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
The default logic in dispatchDecidePolicyForMIMEType doesn't quite match the
Mac equivalent implementation.	You can find the Mac implementation in
WebKit/mac/DefaultDelegates/WebDefaultPolicyDelegate.m.  The differences are in
how file:/// are treated.

There is quite a bit of code duplication between new-window-requested and
navigation-requested.  It would be good to remove some of the redundancy here.

If webkit_web_view_real_new_window_requested is the default implementation, I
don't think it needs to have a notImplemented().

There are some coding style issues present too.  In some places you have the {
on the incorrect line, and are missing spaces between "if" and the "(".  There
are places where you have an "else" immediately after an "if" that always
returns.

 703	 // FIXME: add supported mimestypes of plugins

Should probably be "Add mime types supported by plugins."


Alp should probably look over this from an API point of view.