Bug 40201 - Unify fileSystemRepresentation and filenameFromString
Summary: Unify fileSystemRepresentation and filenameFromString
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-04 21:57 PDT by Kwang Yul Seo
Modified: 2010-06-12 21:29 PDT (History)
6 users (show)

See Also:


Attachments
Patch (8.83 KB, patch)
2010-06-04 22:06 PDT, Kwang Yul Seo
ap: review-
ap: commit-queue-
Details | Formatted Diff | Diff
Revised patch (10.57 KB, patch)
2010-06-06 19:38 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff
Revised patch (GTK build fix) (11.09 KB, patch)
2010-06-06 20:04 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kwang Yul Seo 2010-06-04 21:57:23 PDT
fileSystemRepresentation and filenameFromString do the same thing. Unify them into filenameFromString.
Comment 1 Kwang Yul Seo 2010-06-04 22:06:39 PDT
Created attachment 57960 [details]
Patch
Comment 2 Alexey Proskuryakov 2010-06-05 09:52:39 PDT
Comment on attachment 57960 [details]
Patch

filenameFromString() is poorly named and unused/unimplemented on many platforms. It doesn't even return a file name.

I suggest removing this function instead.
Comment 3 Kwang Yul Seo 2010-06-06 19:38:03 PDT
Created attachment 57989 [details]
Revised patch

Okay. Remove filenameFromString instead.
Comment 4 WebKit Review Bot 2010-06-06 19:50:28 PDT
Attachment 57989 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3124176
Comment 5 Kwang Yul Seo 2010-06-06 20:04:12 PDT
Created attachment 57991 [details]
Revised patch (GTK build fix)

GTK build fix.
Comment 6 Alexey Proskuryakov 2010-06-07 10:00:34 PDT
Comment on attachment 57991 [details]
Revised patch (GTK build fix)

Looks good to me as far as most ports are concerned, but I don't understand what Gtk/Efl were doing here. Maybe someone working on Gtk should take a look, too.

-    char* filename = g_uri_unescape_string(string.utf8().data(), 0);
+    char* filename = g_uri_unescape_string(path.utf8().data(), 0);
Comment 7 Kwang Yul Seo 2010-06-07 18:43:49 PDT
(In reply to comment #6)
> (From update of attachment 57991 [details])
> Looks good to me as far as most ports are concerned, but I don't understand what Gtk/Efl were doing here. Maybe someone working on Gtk should take a look, too.
> 
> -    char* filename = g_uri_unescape_string(string.utf8().data(), 0);
> +    char* filename = g_uri_unescape_string(path.utf8().data(), 0);

We can see the reason from the following ChangeLog.

2008-10-15  Marco Barisione  <marco.barisione@collabora.co.uk>

        Reviewed by Holger Freyther.

        http://bugs.webkit.org/show_bug.cgi?id=20664
        [GTK] File names are not always encodable in UTF-8

        On Linux file names are just raw data and cannot always be directly
        encoded in UTF-8 or in any other encodings, so we escape them before
        storing the file name in a String and unescape them before passing
        them to native functions handling files.
Comment 8 WebKit Commit Bot 2010-06-12 21:29:17 PDT
Comment on attachment 57991 [details]
Revised patch (GTK build fix)

Clearing flags on attachment: 57991

Committed r61077: <http://trac.webkit.org/changeset/61077>
Comment 9 WebKit Commit Bot 2010-06-12 21:29:26 PDT
All reviewed patches have been landed.  Closing bug.