After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 583953 - about:recover not implemented
about:recover not implemented
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Backend
git master
Other Linux
: Normal major
: ---
Assigned To: Xan Lopez
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-05-27 00:39 UTC by Diego Escalante Urrelo (not reading bugmail)
Modified: 2009-08-24 16:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Comments "about:recover" functionality (2.40 KB, patch)
2009-07-29 12:27 UTC, Julián de Navascués
accepted-commit_now Details | Review
Warns before recover a possible crashing web (4.33 KB, patch)
2009-08-24 13:45 UTC, Julián de Navascués
none Details | Review
corrected patch (4.25 KB, patch)
2009-08-24 15:48 UTC, Julián de Navascués
none Details | Review

Description Diego Escalante Urrelo (not reading bugmail) 2009-05-27 00:39:08 UTC
ssia
Comment 1 Julián de Navascués 2009-07-29 12:27:10 UTC
Created attachment 139469 [details] [review]
Comments "about:recover" functionality

Because "about:recover:" is not implemented yet, Epiphany cant recover tabs that were loading during a crash. So this patch avoids the attempts to recover
these tabs and therefore blank tabs wont be opened. Tabs that were fully loaded are still opened and loaded succesfully.
Comment 2 Xan Lopez 2009-07-29 13:04:58 UTC
(In reply to comment #1)
> Created an attachment (id=139469) [edit]
> Comments "about:recover" functionality
> 
> Because "about:recover:" is not implemented yet, Epiphany cant recover tabs
> that were loading during a crash. So this patch avoids the attempts to recover
> these tabs and therefore blank tabs wont be opened. Tabs that were fully loaded
> are still opened and loaded succesfully.
> 

I think this is very reasonable as a temporary solution while we don't re-implement about:recover or something equivalent. I'll apply this patch and leave the bug open until we do that, thanks!
Comment 3 Julián de Navascués 2009-08-24 13:45:08 UTC
Created attachment 141555 [details] [review]
Warns before recover a possible crashing web
Comment 4 Xan Lopez 2009-08-24 14:38:32 UTC
Comment on attachment 141555 [details] [review]
Warns before recover a possible crashing web

>+static void 
>+ask_before_reload (EphyWindow* window, char* url, char* title)

I'd call this "confirm_before_loading" I think.

>+{
>+	
>+	GtkWidget *embed; 
>+	GString *html = g_string_new("");
>+	char *message = g_markup_printf_escaped
>+		/* Translators: %s refers to the LSB distributor ID, for instance MandrivaLinux */
>+		(_("This page was loading when the web browser closed unexpectedly. "
>+		   "This might happen again if you reload the page. If it does, please report "
>+		   "the problem to the %s developers."),
>+		   LSB_DISTRIBUTOR);
>+
>+	char *head = g_markup_printf_escaped (_("Blank page"));
>+
>+	gchar *format =  

This can be "const char *"

>+	  "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
>+	  "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">"
>+	  "<html xmlns=\"http://www.w3.org/1999/xhtml\" lang=\"%s\" xml:lang=\"%s\">" /* language (twice) */
>+	  "<head>"
>+	  "<title>"
>+	  "%s" /* web title */
>+	  "</title>"
>+	  "<style type=\"text/css\">"
>+	  "div#body {"
>+	  "top: 12px;"
>+	  "right: 12px;"
>+	  "bottom: 12px;"
>+	  "left: 12px;"
>+	  "overflow: auto;"
>+	  "background-color:#dcdad5;"
>+	  "font: message-box;"
>+	  "border: 1px solid;"
>+	  "background-repeat: no-repeat;"
>+	  "background-position: center left;"
>+	  "padding: 12px 12px 12px 72px;"
>+	  "text-align:left;"
>+		
>+	  "h1 {"
>+	  
>+	  "margin: 0;"
>+	  "font-size: 1.2em;"
>+	  
>+	  "}"
>+	  "</style>"
>+	
>+	  "</head>"
>+
>+	  "<body dir=\"%s\">" /* rtl or ltr */
>+	  "<div id=\"body\">"
>+
>+	  "<h1>"
>+	  "%s" /* head of the message */
>+	  "</h1>"
>+
>+	  "<p> %s </p>"   /* message */
>+
>+	  "</div>"
>+	  "</body>"
>+	  "</html>";
>+
>+	char *language = g_strdup (pango_language_to_string (gtk_get_default_language ()));
>+	g_strdelimit (language, "_-@", '\0');
>+
>+	g_string_printf (html, format, language, language, title,
>+			   gtk_widget_get_default_direction () == GTK_TEXT_DIR_RTL ? "rtl" : "ltr", 
>+			   head, message);

So 'head' here will be always 'Blank Page'? Why is that there?

Also, the indentation is wrong, the parameters have to align.

>+
>+  	embed  = (GtkWidget*) ephy_shell_new_tab (ephy_shell, window, NULL, NULL, 
>+				     EPHY_NEW_TAB_IN_EXISTING_WINDOW | EPHY_NEW_TAB_APPEND_LAST);

Indentation again.

>+
>+	/* show generated html and put the original URL in the navigation bar */
>+	webkit_web_view_load_html_string ( (WebKitWebView*) EPHY_GET_EPHY_WEB_VIEW_FROM_EMBED (embed),

There's EPHY_GET_WEBKIT_WEB_VIEW_FROM_EMBED, you can use that and lose the cast.

>+					   html->str, 
>+					   (gchar*) url);	

You don't need to cast url, it's already char*.

>+
>+	g_string_free (html, TRUE);
>+	g_free (message);

You are leaking the language variable.

>+
>+}
>+
>+
>+static void 
> parse_embed (xmlNodePtr child,
> 	     EphyWindow *window,
> 	     EphySession *session)
>@@ -1289,10 +1372,15 @@ parse_embed (xmlNodePtr child,
> 			xmlFree (attr);
> 
> 			url = xmlGetProp (child, (const xmlChar *) "url");
>-			if (url == NULL) continue;
>+			if (url == NULL) 
>+			  continue;
>+
>+			/* in the case that crash happens before we receive the URL from the server,
>+			   this will open an about:blank tab. See http://bugzilla.gnome.org/show_bug.cgi?id=591294
> 
>-			/* about:recover is not implemented, so tabs
>-			   loading during crash wont be recovered (bug #583953) */
>+			   Otherwise, if the web was fully loaded, it is reloaded again.
>+			
>+			*/
> 			if (!was_loading ||
> 			    strcmp ((const char *) url, "about:blank") == 0)
> 			{
>@@ -1302,6 +1390,17 @@ parse_embed (xmlNodePtr child,
> 						    EPHY_NEW_TAB_IN_EXISTING_WINDOW |
> 						    EPHY_NEW_TAB_OPEN_PAGE |
> 						    EPHY_NEW_TAB_APPEND_LAST);
>+			}else 
>+
>+			/* shows a message to the user that warns that this page was
>+			   loading during crash and make Epiphany crash again,
>+			   in this case we know the URL */
>+			if (was_loading && url != NULL &&
>+			    strcmp ((const char *) url, "about:blank") != 0)
>+			{
>+				xmlChar* title = xmlGetProp (child, (const xmlChar *) "title");
>+			
>+				ask_before_reload (window, (char*) url, (char*) title);
> 			}
> 
> 			xmlFree (url);
>-- 
>1.6.0.4
>
Comment 5 Julián de Navascués 2009-08-24 15:48:57 UTC
Created attachment 141569 [details] [review]
corrected patch
Comment 6 Xan Lopez 2009-08-24 16:00:44 UTC
I have committed this on master, thanks! Will be on 2.27.91.