[webkit-reviews] review denied: [Bug 13507] URL parsing from adress input in GdkLauncher : [Attachment 14207] gdklaunch-fixurlparsing.patch

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <bdash at webkit.org> has denied Kulyk Nazar
<schamane at myeburg.net>'s request for review:
Bug 13507: URL parsing from adress input in GdkLauncher
http://bugs.webkit.org/show_bug.cgi?id=13507

Attachment 14207: gdklaunch-fixurlparsing.patch
http://bugs.webkit.org/attachment.cgi?id=14207&action=edit

------- Additional Comments from Mark Rowe (bdash) <bdash at webkit.org>
Please take a look at the WebKit coding style guidelines at
<http://webkit.org/coding/coding-style.html>.  Your change is not consistent
with many of the guidelines there, nor with the style used elsewhere in the
file you modified.

As for the substance of the change, I'm not sure that I understand the need for
many of the cases in the is_url function.  It seems to me that the logic should
be to trim whitespace from the beginning/end of the string, check whether the
string starts with a protocol that WebKit can handle and if not assume it's
intended to be HTTP and prepend http:// to the string.	I can't see the point
in checking for irc:/ftp: nor irc./ftp. at this point as there's no support for
either of those protocols in WebKit.  It seems to me that this patch does
nothing to address the original report: entering osnews.com results in
GTKURL_HOST being returned from is_url, which leads to osnews.com being passed
through to FrameLoader::load as happens with the existing code.