Bug 39944 - [GTK] GtkLauncher should support relative file paths
Summary: [GTK] GtkLauncher should support relative file paths
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-31 02:06 PDT by Martin Robinson
Modified: 2010-11-02 17:14 PDT (History)
4 users (show)

See Also:


Attachments
Patch for this issue (2.13 KB, patch)
2010-05-31 02:36 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch with style and memory leak fix (2.27 KB, patch)
2010-05-31 03:22 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch with fixes (2.08 KB, text/plain)
2010-06-03 17:15 PDT, Martin Robinson
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2010-05-31 02:06:14 PDT
This is supported for WinLauncher. It makes GtkLauncher much easier to use for debugging failing tests.
Comment 1 Martin Robinson 2010-05-31 02:36:06 PDT
Created attachment 57444 [details]
Patch for this issue
Comment 2 WebKit Review Bot 2010-05-31 02:37:01 PDT
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.
Comment 3 Martin Robinson 2010-05-31 02:46:36 PDT
> 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
Comment 4 Martin Robinson 2010-05-31 03:22:47 PDT
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 5 Xan Lopez 2010-06-01 05:11:50 PDT
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);
Comment 6 Xan Lopez 2010-06-01 05:15:04 PDT
Eh, ok, you can't use GOwnPtr ok, so disregard that part.
Comment 7 Xan Lopez 2010-06-01 05:16:13 PDT
I meant GOwnPtr here... you can still keep a single branch since it's ok to call g_free with a NULL pointer though.
Comment 8 Martin Robinson 2010-06-03 17:15:20 PDT
Created attachment 57836 [details]
Patch with fixes
Comment 9 Martin Robinson 2010-06-04 11:01:57 PDT
Thanks for the review! As we discussed recently, GOwnPtr cannot be used in GtkLauncher. I've implemented your other suggestions though.
Comment 10 Xan Lopez 2010-06-04 14:40:39 PDT
Comment on attachment 57836 [details]
Patch with fixes

r=me
Comment 11 Martin Robinson 2010-06-04 15:19:47 PDT
Comment on attachment 57836 [details]
Patch with fixes

Clearing flags on attachment: 57836

Committed r60713: <http://trac.webkit.org/changeset/60713>
Comment 12 Martin Robinson 2010-06-04 15:19:53 PDT
All reviewed patches have been landed.  Closing bug.