GNOME Bugzilla – Bug 503968
epiphany-webkit: view page source does not work
Last modified: 2009-09-02 19:41:32 UTC
here is another issue with the webkit backend. the view page source feature in epiphany-webkit does not work. ctrl+u and using the view menu "page source" option do not do anything. thanks for the hard work.
this is debian bug 452764 (http://bugs.debian.org/452764).
See also bug 562446. WebKitGtk now provides the Web Inspector. This could be used to view sources too. You can't edit or save, but maybe is better then a plain Text Editor.
So, should the Ctrl-U shortcat and the "Page Source" menu_item associated action take care of opening the web inspector? I've seen the web inspector is now in trunk, but disabled by default. In case the answer to my question from above was "yes", would it make sense to hide/show disable/enable both the shortcut and the menu item depending on the status of the Web inspector (disabled/enabled)? I thought of it since having the "Page Source" item visible but doing nothing looks not like a good or desirable thing. Just thinking aloud, trying to give some ideas :-)
I don't think it should open the inspector, it should open the page source in $EDITOR like it used to do. Now that we have download support I think it should be easy to reimplement this, IIRC we were just downloading the source manually before with EphyEmbedPersist (you'd use WebKitDownload now).
Ok, so you're proposing to keep separated the source viewer and the inspector, in the same way mozilla does it with the "view source" option and its DOM inspector.Makes sense to me as well. I'll try to take a look on this issue, following these comments, when I have some time. Eventually I could even come out with a patch (hope so) Thanks for the feedback
*** Bug 587608 has been marked as a duplicate of this bug. ***
Created attachment 141185 [details] [review] [PATCH] Use DataSource API to provide view page source src/window-commands.c | 159 +++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 135 insertions(+), 24 deletions(-)
Comment on attachment 141185 [details] [review] [PATCH] Use DataSource API to provide view page source This patch depends on the DataSource API being committed to WebKitGTK+ (https://bugs.webkit.org/show_bug.cgi?id=24758). We could use the Download API, but I think this is a better way to solve this - we are seeing exactly what the browser already downloaded.
Comment on attachment 141185 [details] [review] [PATCH] Use DataSource API to provide view page source > static void >-save_source_completed_cb (EphyEmbedPersist *persist) >+save_temp_source_error_response_cb (GtkWidget *dialog, gint response_id, gpointer user_data) > { >- const char *dest; >- guint32 user_time; >+ gtk_widget_destroy (dialog); >+} >+ >+static void >+save_temp_source_write_display_error (GError *error, EphyEmbed *embed) >+{ >+ EphyWebView *view; >+ GtkWidget *parent_window; >+ GtkWidget *dialog; >+ >+ view = ephy_embed_get_web_view (embed); >+ parent_window = gtk_widget_get_toplevel (GTK_WIDGET (view)); >+ >+ if (!GTK_WIDGET_TOPLEVEL (parent_window)) >+ parent_window = NULL; >+ >+ dialog = gtk_message_dialog_new_with_markup (GTK_WINDOW (parent_window), >+ GTK_DIALOG_DESTROY_WITH_PARENT, >+ GTK_MESSAGE_ERROR, >+ GTK_BUTTONS_CLOSE, >+ _("<b>Failure creating temporary file.</b>")); >+ >+ gtk_message_dialog_format_secondary_markup (GTK_MESSAGE_DIALOG (dialog), >+ _("Saving the current page's contents to a " >+ "temporary file for display was not possible:\n" >+ "%s"), error->message); We won't be able to get this in with string freeze. Isn't there any generic string you can use here? Also, I hate dialogs. GtkInfoBar? >+ >+ g_signal_connect (dialog, "response", G_CALLBACK (save_temp_source_error_response_cb), NULL); >+ >+ gtk_widget_show_all (dialog); >+} >+ >+static void >+save_temp_source_close_cb (GOutputStream *ostream, GAsyncResult *result, GString *data) >+{ >+ char *uri; > GFile *file; >+ GError *error = NULL; > >- user_time = ephy_embed_persist_get_user_time (persist); >- dest = ephy_embed_persist_get_dest (persist); >- g_return_if_fail (dest != NULL); >+ g_output_stream_close_finish (ostream, result, &error); >+ if (error) { >+ EphyEmbed *embed = (EphyEmbed*)g_object_get_data (G_OBJECT (ostream), >+ "ephy-save-temp-source-embed"); >+ save_temp_source_write_display_error (error, embed); >+ g_error_free (error); >+ return; >+ } > >- file = g_file_new_for_path (dest); >- ephy_file_delete_on_exit (file); >+ uri = (char*)g_object_get_data (G_OBJECT (ostream), "ephy-save-temp-source-uri"); >+ file = g_file_new_for_uri (uri); >+ ephy_file_launch_handler ("text/plain", file, gtk_get_current_event_time ()); > >- ephy_file_launch_handler ("text/plain", file, user_time); >+ g_free (uri); > g_object_unref (file); > } > > static void >+save_temp_source_write_cb (GOutputStream *ostream, GAsyncResult *result, GString *data) >+{ >+ GError *error = NULL; >+ gssize written; >+ >+ written = g_output_stream_write_finish (ostream, result, &error); >+ if (error) { >+ g_string_free (data, FALSE); why FALSE? You are leaking the string. >+ g_critical ("Unable to write to file: %s", error->message); >+ g_error_free (error); You are not closing the stream or freeing the data on error, not good. >+ return; >+ } >+ >+ if (written == data->len) { >+ g_output_stream_close_async (ostream, G_PRIORITY_DEFAULT, NULL, >+ (GAsyncReadyCallback)save_temp_source_close_cb, >+ NULL); You pass nothing to the callback, but the callback is defined as if it would get the data. Also, you leak it :) >+ return; >+ } >+ >+ data->len -= written; >+ data->str += written; >+ >+ g_output_stream_write_async (ostream, >+ data->str, data->len, >+ G_PRIORITY_DEFAULT, NULL, >+ (GAsyncReadyCallback)save_temp_source_write_cb, >+ data); >+} >+ >+static void >+save_temp_source_replace_cb (GFile *file, GAsyncResult *result, EphyEmbed *embed) >+{ >+ EphyWebView *view; >+ WebKitWebFrame *frame; >+ WebKitWebDataSource *data_source; >+ const GString *const_data; No need for this to be const, the API was changed (because it didn't make much sense). >+ GString *data; >+ GFileOutputStream *ostream; >+ GError *error = NULL; >+ >+ ostream = g_file_replace_finish (file, result, &error); >+ if (error) { >+ EphyEmbed *embed = (EphyEmbed*)g_object_get_data (G_OBJECT (ostream), >+ "ephy-save-temp-source-embed"); Hmm... who sets this data exactly? Can't you use the parameter you get? (I guess it's a copy&paste mistake) >+ save_temp_source_write_display_error (error, embed); >+ g_error_free (error); >+ return; >+ } >+ >+ g_object_set_data (G_OBJECT (ostream), >+ "ephy-save-temp-source-uri", >+ g_file_get_uri (file)); >+ >+ g_object_set_data (G_OBJECT (ostream), >+ "ephy-save-temp-source-embed", >+ embed); >+ >+ view = ephy_embed_get_web_view (embed); >+ frame = webkit_web_view_get_main_frame (WEBKIT_WEB_VIEW (view)); >+ data_source = webkit_web_frame_get_data_source (frame); >+ const_data = webkit_web_data_source_get_data (data_source); >+ >+ /* We create a new GString here because we need to make sure >+ * we keep writing in case of partial writes */ >+ data = g_string_sized_new (const_data->len); >+ data->str = const_data->str; >+ data->len = const_data->len; Mmm, just data = g_string_new_len (const_data->str, const_data->len); ? (probably g_string_new (const_data->str) would be OK too, since the source of a page will be a string anyway). >+ >+ g_output_stream_write_async (G_OUTPUT_STREAM (ostream), >+ data->str, data->len, >+ G_PRIORITY_DEFAULT, NULL, >+ (GAsyncReadyCallback)save_temp_source_write_cb, >+ data); >+} >+ >+static void
And btw, I'd totally dig a gconf key to make view source use the built-in thing webkit has. I'd use it :D
How was this handled in Epiphany/Gecko? I don't think we should bother the user with temporary file errors. Such a message should be logged to the console at best. The error message should suggest a possible solution such as "Couldn't display the page source. Please free some disk space and try again." If you know what kind of errors to expect beforehand, you could even adjust the suggested solution based upon that.
(In reply to comment #9) > We won't be able to get this in with string freeze. Isn't there any generic > string you can use here? I can try to find one. > Also, I hate dialogs. GtkInfoBar? I'll see what I can do =). > >+ written = g_output_stream_write_finish (ostream, result, &error); > >+ if (error) { > >+ g_string_free (data, FALSE); > > why FALSE? You are leaking the string. Because of this: > >+ /* We create a new GString here because we need to make sure > >+ * we keep writing in case of partial writes */ > >+ data = g_string_sized_new (const_data->len); > >+ data->str = const_data->str; > >+ data->len = const_data->len; > > Mmm, just data = g_string_new_len (const_data->str, const_data->len); ? > > (probably g_string_new (const_data->str) would be OK too, since the source of a > page will be a string anyway). I didn't want to copy the string needlessly. g_string_new_len will copy const_data->str, while what I did just uses what is there in the GString that is returned. So I'm not leaking the string, but we can make it be a copy if you think it's better. > >+ g_critical ("Unable to write to file: %s", error->message); > >+ g_error_free (error); > > You are not closing the stream or freeing the data on error, not good. Yeha, missing closing the stream. > >+ if (written == data->len) { > >+ g_output_stream_close_async (ostream, G_PRIORITY_DEFAULT, NULL, > >+ (GAsyncReadyCallback)save_temp_source_close_cb, > >+ NULL); > > You pass nothing to the callback, but the callback is defined as if it would > get the data. Also, you leak it :) Yeah, I have fixed both yesterday while reviewing the patch =). > >+ GString *data; > >+ GFileOutputStream *ostream; > >+ GError *error = NULL; > >+ > >+ ostream = g_file_replace_finish (file, result, &error); > >+ if (error) { > >+ EphyEmbed *embed = (EphyEmbed*)g_object_get_data (G_OBJECT (ostream), > >+ "ephy-save-temp-source-embed"); > > Hmm... who sets this data exactly? Can't you use the parameter you get? (I > guess it's a copy&paste mistake) copy/paste mistake, yeah
I committed a slightly modified patch after discussing the remaining problems with Xan on IRC to master, will be in the next version.
There aren't interested thing. On my terminal I saw: " (epiphany-webkit:4344): GLib-GIO-WARNING **: Could not initialize inotify ** (epiphany-webkit:4344): WARNING **: Cannot extract frame (0, 0) from the grid "
In Web Inspector page source is showing.