[GTK] Remove platform specific implementation of KURL::fileSystemPath()
Created attachment 196815 [details] Patch
There is one thing to be considered here: the GTK+ implementation of KRUL::fileSystemPath ends up calling FileSystemGtk's filenameToString(). That code uses glib's file naming functions which are smart enough to deal with systems that use encodings other than UTF-8 and even names that cannot be encoded to anything. We will lose that feature by adopting the shared implementation. I don't mind, but I'd like to hear other opinions.
(In reply to comment #2) > That code uses glib's file naming functions which are smart enough to deal with systems that use encodings other than UTF-8 and even names that cannot be encoded to anything. For what is the encoding stuff needed? Input of KRUL::fileSystemPath is UTF16 and output is UTF16? Any special encondig of the filesytem has to be done somewhere else (e.g. at http://trac.webkit.org/browser/trunk/Source/WebCore/platform/gtk/FileSystemGtk.cpp?rev=148014#L56) IMHO.
(In reply to comment #2) > There is one thing to be considered here: the GTK+ implementation of KRUL::fileSystemPath ends up calling FileSystemGtk's filenameToString(). That code uses glib's file naming functions which are smart enough to deal with systems that use encodings other than UTF-8 and even names that cannot be encoded to anything. We will lose that feature by adopting the shared implementation. I don't mind, but I'd like to hear other opinions. The old code looks a bit wrong too: RefPtr<GFile> file = adoptGRef(g_file_new_for_path(path().utf8().data())); GOwnPtr<char> filename(g_file_get_path(file.get())); return filenameToString(filename.get()); Here it's using a UTF-8 string to produce a filename, while it should instead be using a system-encoded string. As long as translation to a system-encoded string happens later, I think it might be okay to use the shared implementation.
(In reply to comment #4) > (In reply to comment #2) > The old code looks a bit wrong too: > > RefPtr<GFile> file = adoptGRef(g_file_new_for_path(path().utf8().data())); > GOwnPtr<char> filename(g_file_get_path(file.get())); > return filenameToString(filename.get()); > > Here it's using a UTF-8 string to produce a filename, while it should instead be using a system-encoded string. As long as translation to a system-encoded string happens later, I think it might be okay to use the shared implementation. The more importent failure is, that path() returns a percent-encoded string and needs a decodeURLEscapeSequences(). So this code never worked with non-ASCII file names anyway. So what about landing it?
Comment on attachment 196815 [details] Patch Clearing flags on attachment: 196815 Committed r148440: <http://trac.webkit.org/changeset/148440>
All reviewed patches have been landed. Closing bug.