I hope it's ok to add something to a bug I reported myself - but I
found out that Galeon has the same problem, maybe this helps:

http://bugzilla.gnome.org/show_bug.cgi?id=59572

So, once they fix it, maybe the fix can be copied, or the other way
around. I can only say, this definitely works in Mozilla, and it
definitely is a bug, leaving technical aspects aside. From my (user)
view, when I click a link, and it works when left-clicking, it
absolutely should also work when opening in a new tab/window instead,
by middle-clicking.

Yeah it's just not implemented. I filed an rfe to gtkmozembed to add a
referrer argument to load_url a while ago but it's not fixed. Maybe we
should go ahead and use mozilla api directly ...

*** Bug 145704 has been marked as a duplicate of this bug. ***

-> Mozilla interaction

Mass reassigning of Epiphany bugs to epiphany-maint@b.g.o

*** Bug 302533 has been marked as a duplicate of this bug. ***

why is this still not fixed?

*** Bug 313980 has been marked as a duplicate of this bug. ***

Updating version fields. The relevent mozilla bug is
https://bugzilla.mozilla.org/show_bug.cgi?id=135659

There's a patch for mozbug 135659 now that needs testing...

*** Bug 353373 has been marked as a duplicate of this bug. ***

Just noting that this problem also happens with the webkit from trunk as of today. I'll investigate.

A quick look tells me that we probably want to have this patch applied to WebKit/GTK+: https://bugs.webkit.org/show_bug.cgi?id=18608.

Along with that, we should refactor ephy_shell_new_tab to accept a WebKitNetworkRequest instead of an URI, and use webkit_web_frame_load_request instead of webkit_web_view_open, which will gain us Referer and any other headers which would be used by webkit for a normal load, instead of we doing a 'faked' load.

I have to test this notion, but I believe this will work.

Created attachment 125356 [details] [review]
proposed fix for webkit

With current WebKit this patch has two problems:

1. It doesn't really fix the problem it is supposed to fix =); this is because WebKitNetworkRequest is not finished in current WebKit trunk, and only carries the URI; we need the patch in https://bugs.webkit.org/show_bug.cgi?id=18608 applied.

2. It no longer does "normalization" (adding http://) of URI's; it used to do that in impl_load, but since the URI is not reaching it anymore, I'm not sure it is the correct place to do it; should WebKitNetworkRequest do the normalization when the URI is being set?

Created attachment 125402 [details] [review]
proposed patch for webkit

ok, so this is what this patch does:

  1. uses WebKitNetworkRequest when openning pages
  2. removes load flags; these were aparently used only to decide on whether gecko should be allowed to do its own "normalization" of the URL, and is thus deprecated
  3. moves URL normalization from impl_load to ephy_link_open, since the former now receives a WebKitNetworkRequest

Looks pretty good (and scary too, we are finally adding WebKit specific stuff to EphyEmbed... ;)), just some nitpicks:

-  webkit_web_view_open (wembed->priv->web_view, effective_url);
-
-  g_free (effective_url);
+  webkit_web_frame_load_request(main_frame, request);

space between function name and parameters.

+	embed = ephy_shell_new_tab_full (shell, parent_window,
+									 previous_embed, request, flags,
+									 EPHY_EMBED_CHROME_ALL, FALSE, 0);
+	

Seems indentation got wrong here.

As commented on IRC, I think it's best if we don't add webkit-only stuff to the Embed interface, and put this code somewhere else. So we'll create EphyWebView inheriting from WebKitWebView to add things such as this.

Created attachment 126491 [details] [review]
Adding the new EphyWebView object

The object had to be stuck in embed/ because EphyEmbed will need to use it, and it seemed to me that a dependency from embed to src/ was not intended. This is of course something we will want to move to src/ when possible.

Created attachment 126492 [details] [review]
This patch adds loading API to the new object

As you can see, I decided to format this work as a patch set. This is because I'm touching multiple issues at once, and because you can also decide to allow me to commit some of them and fix the others for another review round (each one becomes a commit which makes more sense to Ephy's history IMO).

Created attachment 126493 [details] [review]
Actual fix

Finally, the patch that would fix this issue. I have tested this with my NetworkRequest patch applied to WebKit, and it works, so we better decide if we want to expose SoupMessage in NetworkRequest and get that patch landed, too ;). I could also prepare a smaller patch that only makes NetworkRequest carry the ResourceRequest around, which would be enough for this one.

Created attachment 126494 [details] [review]
Pull URL normalization up in the chain

This patch is needed so that stuff that use the new and modified code paths will also work when, for instance, URLs are typed in the location bar without 'http://'.

Created attachment 126495 [details] [review]
Remove unused LoadFlags

Finally, this is a clean-up patch to remove the unused (and unneeded for WebKit) LoadFlags.

(In reply to comment #18)
> Created an attachment (id=126491) [edit]
> Adding the new EphyWebView object
> 
> The object had to be stuck in embed/ because EphyEmbed will need to use it, and
> it seemed to me that a dependency from embed to src/ was not intended. This is
> of course something we will want to move to src/ when possible.
> 

This seems OK, but please kill all the useless boilerplate before landing (you only need empty class_init and init).

(In reply to comment #19)
> Created an attachment (id=126492) [edit]
> This patch adds loading API to the new object
> 
> As you can see, I decided to format this work as a patch set. This is because
> I'm touching multiple issues at once, and because you can also decide to allow
> me to commit some of them and fix the others for another review round (each one
> becomes a commit which makes more sense to Ephy's history IMO).
> 

This is mising documentation for the function and checking of the parameters, since it's public API. Otherwise it's OK.

(In reply to comment #20)

+		ephy_web_view_load_request (EPHY_WEB_VIEW (EPHY_GET_WEBKIT_WEB_VIEW_FROM_EMBED (embed)),
+                                            request);

Indentation.

+	embed = ephy_shell_new_tab_full (shell, parent_window,
+                                         previous_embed, request, flags,
+                                         EPHY_EMBED_CHROME_ALL, FALSE, 0);

Indentation.

-		if (flags & EPHY_LINK_ALLOW_FIXUP)
-		{
-			ephy_embed_load (embed, 
-					 address, 
-					 EPHY_EMBED_LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP,
-					 NULL);
-		}
-		else
-		{
-			ephy_embed_load_url (embed, address);
-		}
+		ephy_embed_load_url (embed, address);

Why is this in this patch?

(In reply to comment #21)
> Created an attachment (id=126494) [edit]
> Pull URL normalization up in the chain
> 
> This patch is needed so that stuff that use the new and modified code paths
> will also work when, for instance, URLs are typed in the location bar without
> 'http://'.
> 

I've changed this code a bit in upstream, it now uses a ephy-embed-utils to check for proper URLs. Can do you update the patch to do the same?

(In reply to comment #22)
> Created an attachment (id=126495) [edit]
> Remove unused LoadFlags
> 
> Finally, this is a clean-up patch to remove the unused (and unneeded for
> WebKit) LoadFlags.
> 

Makes sense, but unless I'm missing something you should add stuff from previous patches here. 

Created attachment 135561 [details] [review]
Actual fix

(In reply to comment #28)
> Created an attachment (id=135561) [edit]
> Actual fix
> 

OK.

Created attachment 135562 [details] [review]
126494: Pull URL normalization up in the chain

Created attachment 135563 [details] [review]
Remove unused LoadFlags

These are now committed. The change we depend upon for the fix to actually work is already in WebKitGTK+'s svn, and will be in the 1.1.9 release. Yay!