Bug 25808 - Create file uri from filename properly
Summary: Create file uri from filename properly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-14 14:43 PDT by Fridrich Strba
Modified: 2009-05-15 16:50 PDT (History)
1 user (show)

See Also:


Attachments
use a proper conversion from filename to uri (1.32 KB, patch)
2009-05-14 14:44 PDT, Fridrich Strba
gustavo: review-
Details | Formatted Diff | Diff
changed the condition as for review (1.33 KB, patch)
2009-05-14 15:09 PDT, Fridrich Strba
no flags Details | Formatted Diff | Diff
A clean standard-complying patch with changelog (2.61 KB, patch)
2009-05-15 00:17 PDT, Fridrich Strba
no flags Details | Formatted Diff | Diff
This patch even has the proper e-mail address and spaces instead of tabs (2.62 KB, patch)
2009-05-15 01:44 PDT, Fridrich Strba
no flags Details | Formatted Diff | Diff
Adding the WebKit/gtk/tests/testdownload.c file to the patch (3.49 KB, patch)
2009-05-15 09:48 PDT, Fridrich Strba
jmalonzo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fridrich Strba 2009-05-14 14:43:43 PDT
A simple concatenating of file:// with the filename string is not valid file -> uri conversion. Use g_filename_to_uri instead because it does the right thing even on windows.
Comment 1 Fridrich Strba 2009-05-14 14:44:24 PDT
Created attachment 30358 [details]
use a proper conversion from filename to uri
Comment 2 Gustavo Noronha (kov) 2009-05-14 14:50:16 PDT
Comment on attachment 30358 [details]
use a proper conversion from filename to uri

> +#ifndef _WIN32
>      // we avoid the escaping for local files, because
>      // g_filename_from_uri (used internally by GFile) has problems
>      // decoding strings with arbitrary percent signs
>      if (url.isLocalFile())
>          d->m_gfile = g_file_new_for_path(url.prettyURL().utf8().data() + sizeof("file://") - 1);
>      else
> +#endif

I think you want to use #if !PLATFORM(WIN_OS) here?

Looks fine otherwise. Also, add a ChangeLog entry, please.
Comment 3 Fridrich Strba 2009-05-14 15:09:33 PDT
Created attachment 30361 [details]
changed the condition as for review

Will add changelog soon
Comment 4 Fridrich Strba 2009-05-15 00:17:42 PDT
Created attachment 30367 [details]
A clean standard-complying patch with changelog
Comment 5 Fridrich Strba 2009-05-15 01:44:47 PDT
Created attachment 30377 [details]
This patch even has the proper e-mail address and spaces instead of tabs
Comment 6 Jan Alonzo 2009-05-15 08:29:09 PDT
(In reply to comment #5)
> Created an attachment (id=30377) [review]
> This patch even has the proper e-mail address and spaces instead of tabs
> 

Hi Fridrich

Looking good. Can you also include WebKit/gtk/tests/testdownload.c in this patch? 

Thanks.
Comment 7 Fridrich Strba 2009-05-15 09:48:04 PDT
Created attachment 30391 [details]
Adding the WebKit/gtk/tests/testdownload.c file to the patch
Comment 8 Jan Alonzo 2009-05-15 16:47:57 PDT
Comment on attachment 30391 [details]
Adding the WebKit/gtk/tests/testdownload.c file to the patch

r=me
Comment 9 Jan Alonzo 2009-05-15 16:49:05 PDT
Comment on attachment 30377 [details]
This patch even has the proper e-mail address and spaces instead of tabs

Clearing review flags as this patch is obsolete.
Comment 10 Jan Alonzo 2009-05-15 16:50:48 PDT
(In reply to comment #8)
> (From update of attachment 30391 [details] [review])
> r=me
> 

Landed in r43791