Bug 25808 – Create file uri from filename properly

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Fridrich Strba]

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.

[Fridrich Strba]

Created attachment 30358 [details]
use a proper conversion from filename to uri

[Gustavo Noronha (kov)]

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.

[Fridrich Strba]

Created attachment 30361 [details]
changed the condition as for review

Will add changelog soon

[Fridrich Strba]

Created attachment 30367 [details]
A clean standard-complying patch with changelog

[Fridrich Strba]

Created attachment 30377 [details]
This patch even has the proper e-mail address and spaces instead of tabs

[Jan Alonzo]

(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.

[Fridrich Strba]

Created attachment 30391 [details]
Adding the WebKit/gtk/tests/testdownload.c file to the patch

[Jan Alonzo]

Comment on attachment 30391 [details]
Adding the WebKit/gtk/tests/testdownload.c file to the patch

r=me

[Jan Alonzo]

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.

[Jan Alonzo]

(In reply to comment #8)
> (From update of attachment 30391 [details] [review])
> r=me
> 

Landed in r43791