[Webkit-unassigned] [Bug 41441] createWindow method should only do window-creating without URL navigator

[bugzilla-daemon at webkit.org]

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





--- Comment #30 from Xianzhu Wang <phnixwxz at gmail.com>  2011-01-08 07:01:54 PST ---
(From update of attachment 74080)
View in context: https://bugs.webkit.org/attachment.cgi?id=74080&action=review

>> WebCore/bindings/generic/BindingDOMWindow.h:89
>> +    KURL completedUrl = url.isEmpty() ? KURL() : completeURL(state, url);
> 
> nit: completedUrl -> completedURL
> 
> acronyms should either be lowercase or UPPERCASE depending on whether they appear at the beginning or elsewhere in a name, respectively.

Done. (now in Source/WebCore/page/DOMWindow.cpp

>> WebKit/chromium/public/WebViewClient.h:85
>>      virtual WebView* createView(WebFrame* creator,
> 
> nit: please add a FIXME comment saying that this method is DEPRECATED.

Done.

>> WebKit/chromium/public/WebViewClient.h:88
>> +    // Sometimes it's much better for client API if a new window starts with a
> 
> nit: insert a new line above this comment.

Done.

>> WebKit/chromium/public/WebViewClient.h:92
>> +    virtual WebView* createView(WebFrame* creator,
> 
> nit: (bikeshed comment) how about re-ordering these parameters so that it is creator, request, name, and then features?  that way it more closely resembles window.open(url, name, features).

Done.

>> WebKit/chromium/public/WebViewClient.h:96
>> +        (void)request;                        
> 
> nit: please just leave the WebURLRequest parameter unnamed so that you don't need
> to write this line.

The request parameter is referenced in the comment of the method, so we should keep it. I added a FIXME here. Eventually this line will be removed.

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