Bug 13507 - URL parsing from adress input in GdkLauncher
Summary: URL parsing from adress input in GdkLauncher
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-26 11:36 PDT by Kulyk Nazar
Modified: 2007-04-27 08:16 PDT (History)
1 user (show)

See Also:


Attachments
gdklaunch-fixurlparsing.patch (1.92 KB, patch)
2007-04-26 11:37 PDT, Kulyk Nazar
mrowe: review-
Details | Formatted Diff | Diff
gdklauncher-urlparsing-0.1.patch (1.23 KB, patch)
2007-04-27 05:27 PDT, Kulyk Nazar
no flags Details | Formatted Diff | Diff
gdklauncher-urlparsing-0.2.patch (1.22 KB, patch)
2007-04-27 05:50 PDT, Kulyk Nazar
no flags Details | Formatted Diff | Diff
gdklauncher-urlparsing-0.5.patch (2.49 KB, patch)
2007-04-27 07:38 PDT, Kulyk Nazar
no flags Details | Formatted Diff | Diff
gdklauncher-urlparsing-0.7.patch (2.53 KB, patch)
2007-04-27 07:58 PDT, Kulyk Nazar
mrowe: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kulyk Nazar 2007-04-26 11:36:17 PDT
it cause a lot of curl error if you request not full URL.

Reproduce: start gdklauncher and input some url in to adressline, like : osnews.com without "http://" it opens page, but fails on downloading css and images.
Comment 1 Kulyk Nazar 2007-04-26 11:37:08 PDT
Created attachment 14207 [details]
gdklaunch-fixurlparsing.patch

This patch should do initial parsing for url input.
Comment 2 Mark Rowe (bdash) 2007-04-26 22:00:51 PDT
Comment on attachment 14207 [details]
gdklaunch-fixurlparsing.patch

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.
Comment 3 Kulyk Nazar 2007-04-27 05:27:44 PDT
Created attachment 14223 [details]
gdklauncher-urlparsing-0.1.patch

Initial patch for gdklauncher to parse URLs. Please check if something like this is ok.
Comment 4 Kulyk Nazar 2007-04-27 05:50:24 PDT
Created attachment 14224 [details]
gdklauncher-urlparsing-0.2.patch

Fix for review
Comment 5 Kulyk Nazar 2007-04-27 07:38:31 PDT
Created attachment 14225 [details]
gdklauncher-urlparsing-0.5.patch

Initial work for url correction.
Comment 6 Kulyk Nazar 2007-04-27 07:58:55 PDT
Created attachment 14226 [details]
gdklauncher-urlparsing-0.7.patch

i hope this is last try :)
Comment 7 Mark Rowe (bdash) 2007-04-27 08:06:36 PDT
Comment on attachment 14226 [details]
gdklauncher-urlparsing-0.7.patch

I'm going to say r=me, but there is one minor change I will make when I land this: I'll allow https:// and file:// URLs through unchanged in the same manner as http:// and ftp://.
Comment 8 Mark Rowe (bdash) 2007-04-27 08:16:07 PDT
Landed in r21148 with some minor tweaks.