Summary: | Make the FileSystemChooserGdk implementation properly convert the WebCore::String presentation | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Holger Freyther <zecke> | ||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | Keywords: | Gtk | ||||||
Priority: | P2 | ||||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
Attachments: |
|
Description
Holger Freyther
2007-09-15 07:20:05 PDT
Created attachment 16297 [details]
Properly convert the name of the file
This should implement the NULL-checking and should properly convert from WebCore::String -> UTF-8 -> filesystem and from filesystem -> UTF-8 -> WebCore::String.
Comment on attachment 16297 [details]
Properly convert the name of the file
+ gchar* basename = g_filename_to_utf8(basenameSystem, -1, 0, 0, 0);
+ g_free(basenameSystem);
+
+ if (basename) {
+ string = String::fromUTF8(basename);
+ g_free(basename);
+ }
This sequence should be factored out into a static helper function instead of repeating it twice in this file. Otherwise looks good.
Created attachment 16304 [details]
Convert properly
This takes the comments of Adam from this bug report and irc into account.
Comment on attachment 16304 [details]
Convert properly
I think "convertToStringByAdoptingTheFilesystemRepresentation" may be a little bit wordy. At the least, I think "The" does not add anything. Elsewhere we also treat "filesystem" as two words. Perhaps "stringWithFileSystemRepresentation" would be a better name?
"filenameSystem" also feels backwards. "systemFilename" reads better to me.
Other than these naming niggles, r=me.
(In reply to comment #4) > (From update of attachment 16304 [details] [edit]) > I think "convertToStringByAdoptingTheFilesystemRepresentation" may be a little > bit wordy. At the least, I think "The" does not add anything. Elsewhere we > also treat "filesystem" as two words. Perhaps > "stringWithFileSystemRepresentation" would be a better name? > > "filenameSystem" also feels backwards. "systemFilename" reads better to me. > > Other than these naming niggles, r=me. > Oops, I landed that without reading that part. I agree with systemFilename and will do a fixup (without going through review) but the function name is difficult as it takes ownership of the systemFilename and we wanted to have that inside the name. |