[Webkit-unassigned] [Bug 41441] Add a target URL parameter to the createView and createWindow method

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=41441


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #60230|review?                     |review-
               Flag|                            |




--- Comment #6 from Darin Adler <darin at apple.com>  2010-07-01 12:05:36 PST ---
(From update of attachment 60230)
> +    KURL completedUrl =
> +        url.isEmpty() ? KURL(ParsedURLString, "") : completeURL(url);

It should be completedURL, not completedUrl. See the coding style document and note what you yourself did below with targetURL.

I know you just moved the code, but is KURL(ParsedURLString, "") better than KURL()? Is it different at all?

>      Frame* createWindow(FrameLoader* frameLoaderForFrameLookup, const FrameLoadRequest&, const WindowFeatures&, bool& created);
> +    // Another version of createWindow which passes the target URL where the created window will be navigated to.
> +    Frame* createWindow(FrameLoader* frameLoaderForFrameLookup, const FrameLoadRequest&, const WindowFeatures&, const KURL& targetURL, bool& created);

Do we really need to have both of these? Can we change the callers to all call the new one? I don't like having an extra function here.

>          Page* createWindow(Frame*, const FrameLoadRequest&, const WindowFeatures&) const;
> +        Page* createWindow(Frame*, const FrameLoadRequest&, const WindowFeatures&, const KURL&) const;

Do we really need to have both of these? Can we change the callers to all call the new one? I don't like having an extra function here.

Otherwise seems fine.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.