GNOME Bugzilla – Bug 658028
web-apps: .desktop file creation fails if the name includes a filesystem forbidden character
Last modified: 2012-03-14 12:53:49 UTC
I think we might want to use the domain instead of the page title in ".gnome2/epiphany/app-<name>/". Agree? I'll cook a patch.
I don't really agree no, I think title works much better by default. We should just try to not create names that won't work. Can you give examples?
"Twitter / Home" triggered this for me, because of the "/". AFAICT glib has no API to clean filesystem unfriendly characters. It can URI encode or Markup encode, but nothing for illegal filesystem names.
Created attachment 196123 [details] [review] web apps: Remove G_DIRECTORY_SEPARATOR chars from filenames To avoid nasty surprises
Created attachment 196124 [details] [review] Convert app names to the proper encoding before using them as filenames
These patches depend on the one in bug 658010
Review of attachment 196123 [details] [review]: OK.
Review of attachment 196124 [details] [review]: Since we are bothering to do this I guess we should at least check if there was an error? Dunno, otherwise it's crash in the next line. Also this is borderline "factor this into a method" :P
Comment on attachment 196123 [details] [review] web apps: Remove G_DIRECTORY_SEPARATOR chars from filenames Attachment 196123 [details] pushed as 253893c - web apps: Remove G_DIRECTORY_SEPARATOR chars from filenames
(In reply to comment #7) > Review of attachment 196124 [details] [review]: > > Since we are bothering to do this I guess we should at least check if there was > an error? Dunno, otherwise it's crash in the next line. Maybe I am too naive, but the only reason why this would crash is that we get fed incomplete UTF-8 strings by GTK+, which shouldn't happen in theory.. > Also this is borderline "factor this into a method" :P Yes, I thought about that, but in only two cases it's the same code, for a third one is slightly different.. also, in create_desktop_file() I do need the UTF-8 wm_class anyway.
Created attachment 196272 [details] [review] Convert app names to the proper encoding before using them as filenames Also add proper conversion error handling where needed.
Created attachment 196273 [details] [review] Display an error dialog if the application name is somehow not valid
Review of attachment 196272 [details] [review]: Looks good to me. ::: src/window-commands.c @@ +561,3 @@ if (response == GTK_RESPONSE_OK) { profile_dir = ephy_web_application_get_profile_directory (gtk_entry_get_text (GTK_ENTRY (data->entry))); + if (!profile_dir) { In theory the brace is wrong, although I can see there's another one wrong right before it :D
Review of attachment 196273 [details] [review]: OK, crazy idea. Couldn't we validate th ename before killing the dialog? Showing a dialog after a dialog seems like a sort of bad UI. Perhaps this means a bit more refactoring in the other patch though.
Where are we standing with this? I kinda lost track of it..
Xan: What's the status of this? Asking as the patch still has accepted-commit_after_freeze status...
I think Claudio did not commit the patch after freeze, and for the other one my comments still applies I guess. The entire dialog is going to be redesigned anyway, so perhaps it's worth talking with Lapo at this point. See https://github.com/gnome-design-team/gnome-mockups/blob/master/web/save-as-webapp.png
Created attachment 207095 [details] [review] Convert app names to the proper encoding before using them as filenames Also add proper conversion error handling where needed.
Attachment 207095 [details] pushed as db10ae9 - Convert app names to the proper encoding before using them as filenames
I think everything we wanted to push here has been pushed, closing.