Merge lp:~gue5t/midori/child-frame-errorpages into lp:midori

[gue5t gue5t]

Don't set tab title/special when a non-main frame displays an error

This isn't getting much attention so I'll copy-paste my explanation from the bug report in case that wasn't visible enough:

I think I figured this out. This only happens when the ad loads in an iframe and is blocked in /etc/hosts, leading the frame to display an error page. I opened midori, attached gdb, and set a breakpoint on midori_view_display_error. Then I browsed to <http://www.handletheheat.com/2013/07/the-ultimate-guide-to-chocolate-chip-cookies.html>, with a /etc/hosts entry causing "dg.specificclick.net" to resolve to "0.0.0.0".

gdb showed midori_view_display_error called from webkit_web_view_load_error_cb, which makes sense. In both calls, the web_frame argument indicated the child frame for the ad, not the webview's main frame. Then, the code of midori_view_display_error calls midori_view_set_html, passing the web_frame along. But midori_view_set_html proceeds to, without considering whether it has a child frame or the main frame, run:

katze_item_set_uri (view->item, uri);
midori_tab_set_special (MIDORI_TAB (view), TRUE);

These should only be done if the errored frame is the webview's main frame. I'll reference this from a branch that fixes the bug, but it looks like a similar problem exists under wk2 with a less clear fix.

This branch fixes wk1; wk2 is untested and its behavior unchanged.

[Paweł Forysiuk (tuxator)]

review: Approve

[André Stösel (ivaldi)]

The redirect bug is fixed, but after I restart midori the css for the error page in the frame is missing.

review: Needs Fixing

[gue5t gue5t (gue5t)]

Interesting indeed. I also notice that the page scrolls to the iframe when it loads: this is noticeable if you use a test page that looks like this:

<style>p{height: 100%}</style>
<p></p>
<iframe src="http://mekong:6060/sedus-3ds/" width="90%" height="90%"/>

I'll try to figure out and fix these issues.

[gue5t gue5t (gue5t)]

The page scrolls to the iframe because our error template has the "autofocus" attribute on its button. We'll have to change that to something interpolated into the template only for main-frame error pages.

[gue5t gue5t (gue5t)]

The CSS is missing because midori_view_web_view_resource_request_cb denies access to res:// as follows:
    /* Only apply custom URIs to special pages for security purposes */
    if (!midori_tab_get_special (MIDORI_TAB (view)))
        return;

This checks if the entire tab is an error page, and since it isn't, doesn't allow the frame which needs an error page to load res://about.css. :(

I'll see what can be done.

[André Stösel (ivaldi)]

The bug fix works, the styling issue is no regression and I think this should be merged before release.

(I opened a new bug #1210841 for the styling issue.)

review: Approve

=== modified file 'midori/midori-view.c'
--- midori/midori-view.c	2013-08-02 12:00:42 +0000
+++ midori/midori-view.c	2013-08-02 20:59:28 +0000
@@ -1140,14 +1140,21 @@
     WebKitWebView* web_view = WEBKIT_WEB_VIEW (view->web_view);
     if (!uri)
         uri = "about:blank";
-    katze_item_set_uri (view->item, uri);
-    midori_tab_set_special (MIDORI_TAB (view), TRUE);
 #ifndef HAVE_WEBKIT2
+    WebKitWebFrame* main_frame = webkit_web_view_get_main_frame (web_view);
+    if (web_frame == main_frame)
+    {
+        katze_item_set_uri (view->item, uri);
+        midori_tab_set_special (MIDORI_TAB (view), TRUE);
+    }
     if (!web_frame)
-        web_frame = webkit_web_view_get_main_frame (web_view);
+        web_frame = main_frame;
     webkit_web_frame_load_alternate_string (
         web_frame, data, uri, uri);
 #else
+    /* XXX: with webkit2 ensure child frames do not set tab URI/special/html */
+    katze_item_set_uri (view->item, uri);
+    midori_tab_set_special (MIDORI_TAB (view), TRUE);
     webkit_web_view_load_alternate_html (web_view, data, uri, uri);
 #endif
 }