[webkit-reviews] review denied: [Bug 41441] Add a target URL parameter to the createView and createWindow method : [Attachment 61050] patch V2, change the createWindow method on all ports.

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Johnny Ding
<jnd at chromium.org>'s request for review:
Bug 41441: Add a target URL parameter to the createView and createWindow method
https://bugs.webkit.org/show_bug.cgi?id=41441

Attachment 61050: patch V2, change the createWindow method on all ports.
https://bugs.webkit.org/attachment.cgi?id=61050&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebCore/loader/FrameLoader.h:125
 +	Frame* createWindow(FrameLoader* frameLoaderForFrameLookup, const
FrameLoadRequest&, const WindowFeatures&, const KURL&, bool&);
please do not remove the "created" parameter name.  that was helpful
documentation for the meaning of the bool out parameter.  the rule
in webkit is to leave parameters unnamed if the type name is sufficient
to document its purpose.  in this case, i think you should also give
the KURL parameter a name.  targetURL seems good since you are using
that elsewhere.

actually, it just occurred to me:  why do we need to pass completedURL
when we have the url stored in the frameRequest?  it should not be a
different url.