Summary: | [GTK] GtkLauncher should support relative file paths | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Robinson <mrobinson> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, eric, webkit.review.bot, xan.lopez | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
Martin Robinson
2010-05-31 02:06:14 PDT
Created attachment 57444 [details]
Patch for this issue
Attachment 57444 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/GtkLauncher/main.c:191: Extra space before ( in function call [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:193: Declaration has space between * and variable name in GFile* gfile [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:193: Extra space before ( in function call [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:197: Declaration has space between * and variable name in gchar* full_path [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:197: Extra space before ( in function call [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:197: full_path is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKitTools/GtkLauncher/main.c:201: Declaration has space between * and variable name in gchar* file_url [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:201: Extra space before ( in function call [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:201: file_url is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKitTools/GtkLauncher/main.c:223: Declaration has space between * and variable name in gchar* file_url [whitespace/declaration] [3]
WebKitTools/GtkLauncher/main.c:223: file_url is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKitTools/GtkLauncher/main.c:226: Extra space before ( in function call [whitespace/parens] [4]
WebKitTools/GtkLauncher/main.c:229: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 13 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
> Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 > WebKitTools/GtkLauncher/main.c:191: Extra space before ( in function call [whitespace/parens] Bug for check-webkit-style false positives filed here: https://bugs.webkit.org/show_bug.cgi?id=39947 Created attachment 57449 [details]
Patch with style and memory leak fix
Xan is saying that GtkLauncher should gradually move to WebKit style, so I've redone my patch using WebKit style and with a small memory leak fix.
Comment on attachment 57449 [details] Patch with style and memory leak fix >+static gchar* filenameToURL(const char* filename) >+{ >+ GFile *gFile = g_file_new_for_path(filename); >+ if (!g_file_query_exists(gFile, 0)) { >+ g_object_unref(gFile); >+ return 0; >+ } >+ g_object_unref(gFile); Maybe use g_file_test if you only create the object to check that the path is valid? >+ >+ gchar *fullPath = realpath(filename, 0); >+ if (!fullPath) >+ return 0; >+ >+ gchar *fileURL = g_filename_to_uri(fullPath, 0, 0); >+ free(fullPath); GOwnPtr for fullPath. >+ >+ return fileURL; >+} >+ > int > main (int argc, char* argv[]) > { >@@ -201,8 +221,15 @@ main (int argc, char* argv[]) > main_window = create_window (); > gtk_container_add (GTK_CONTAINER (main_window), vbox); > >- gchar* uri = (gchar*) (argc > 1 ? argv[1] : "http://www.google.com/"); >- webkit_web_view_load_uri (web_view, uri); >+ gchar *uri = (gchar*) (argc > 1 ? argv[1] : "http://www.google.com/"); >+ gchar *fileURL = filenameToURL(uri); >+ >+ if (fileURL) { >+ webkit_web_view_load_uri(web_view, fileURL); >+ g_free(fileURL); >+ } else { >+ webkit_web_view_load_uri(web_view, uri); >+ } You can use GOwnPtr for fileURL, and have only one branch with fileURI ? fileURI : uri > > gtk_widget_grab_focus (GTK_WIDGET (web_view)); > gtk_widget_show_all (main_window); Eh, ok, you can't use GOwnPtr ok, so disregard that part. I meant GOwnPtr here... you can still keep a single branch since it's ok to call g_free with a NULL pointer though. Created attachment 57836 [details]
Patch with fixes
Thanks for the review! As we discussed recently, GOwnPtr cannot be used in GtkLauncher. I've implemented your other suggestions though. Comment on attachment 57836 [details]
Patch with fixes
r=me
Comment on attachment 57836 [details] Patch with fixes Clearing flags on attachment: 57836 Committed r60713: <http://trac.webkit.org/changeset/60713> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/60713 might have broken GTK Linux 32-bit Release The following changes are on the blame list: http://trac.webkit.org/changeset/60712 http://trac.webkit.org/changeset/60713 http://trac.webkit.org/changeset/60709 http://trac.webkit.org/changeset/60710 http://trac.webkit.org/changeset/60711 |