There is an existing command line arg I think.
I'm a bit confused why you would want this and bookmarks opened in
different windows.

I should be more clear :)

If there is an existing window open to www.gnome.org, and I do
"epiphany www.gnome.org" from the command line or using
gnome_url_show, instead of opening another window pointing at
www.gnome.org, raise the existing window that points at www.gnome.org.
Make sense, see the difference?

This would be cool for bookmarks too, creates a 1 to 1 relationship
between views and their object.

I see now :)

Created attachment 28242 [details] [review]
implementation

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

Comment on attachment 28242 [details] [review]
implementation

This patch has bit-rotten.

chpe, any chance the patch can be updated for the current epiphany code? Especially with the deskbar applet this issue becomes more important.

Should be easy to update the patch.

I've got an updated patch coming, but it has a few problems:

First, as far as I can tell ephy-automation.c:impl_ephy_automation_loadUrlWithStartupId() doesn't get called when opening URLs from the command line, so putting an ephy_session_present_window() there doesn't work. I got around that by putting a present_window call inside ephy-session.c:session_command_open_uris(), but I'm not sure if that's right. It works, anyway.

Second, I couldn't figure out how to hook this into opening bookmarks from the Bookmarks menu. It works for the command line/gnome-open and the bookmarks editor, but not the menu itself. I tried to fix that but quickly got lost in a twisty maze of C.

Finally, the strcmp in ephy_session_present_window is kinda fragile. It'll work if your URL is exactly what's in a tab, but it treats gnome.org, http://gnome.org, and http://gnome.org/ etc. as different URLs. 

Anyway, that's the best I could do so far. The following patch should be enough for things like the deskbar applet. I'd be happy to keep working on it if someone wants to point me in the right direction.

(this is my first time working on something other than shell script, please be gentle)

Created attachment 62548 [details] [review]
Updated patch

Thanks for the patch!

The patch doesn't apply as-is to epiphany 2.14 since ephy-automation was removed...
The correct place for this is probably now in ephy-session.c:session_command_open_uris .
For comparing URIs, maybe there's some gnome_vfs_uri_* function to use?

Thanks! I've got an updated patch here that drops the ephy-automation stuff and makes this work for the bookmarks menu too. That takes care of problems 1 and 2, now it's just a matter of making the comparison inside present_window a little smarter. I looked at gnome_vfs_uri_equal(), but that behaves the same as the strcmp does now and just makes things more complicated. I'll keep looking into it.

Created attachment 62584 [details] [review]
Updated patch, take 2

The indentation looks a bit wrong in session_command_open_uris.

+gboolean
+ephy_session_present_window (EphySession *session,
+                const char *address)

I think we need to add a "guint user_time" param here, and use it when presenting the window (gtk_window_present_with_time). In ephy-session, we have a timestamp; otherwise gtk_get_current_event_time() from gtkmain.h should work.

           char *tab_address;
+
+           tab_address = ephy_embed_get_location (ephy_tab_get_embed (tab), TRUE);	
+           found = tab_address != NULL && strcmp (tab_address, address) == 0;

This should use ephy_tab_get_address() which is also guaranteed non-NULL.

Created attachment 63532 [details] [review]
Updated patch, Take 3

Here's a new patch that fixes the last of my concerns, uses ephy_tab_get_address and gtk_window_present_with_time, and (hopefully) fixes the indenting in session_command_open_uris.

I had another version that uses gnome-vfs for the comparison, but that just made things more complicated and basically did the same thing.

+    /* if address doesn't have web prefix, default to http:// */
This code is a bit awkward, but I don't see a better method either... we can't really access gecko's URIFixup from here :/

One thing that might be a bit strange is that when you open a bunch of links in new tabs (--new-tab url1 url2) and some are found in other windows, you'll get a bunch of windows presented instead of all those urls in one window... not sure it's relevant.

Does this patch still apply to svn?

Tim, ping?

Created attachment 87234 [details] [review]
take 4

Terribly sorry about the delay; I missed your first ping.

As far as I can tell take-3.patch still applies to the latest svn (at least I think it is the latest svn, I've never used svn before...), albeit with some minor offset fuzz. I've attached take-4.patch to fix that. 

It seems the take-3.patch still has all the same features/problems it had when I made it, though I'll admit I've only done some quick testing. Most notably, it still treats e.g. www.gnome.org different from gnome.org, but there's probably a million corner cases I haven't even thought of. Trying to cover all of these permutations inside the url comparision could get very messy very quickly. :/

I'd love to work on this some more, but frankly I'm not sure where to go from here.

Thanks for the patch!

The changes made to src/bookmarks/ephy-bookmark-action.c don't belong there; instead they should be put in ephy-window:impl_open, I think.

+		/* Only open a new window if the url isn't already open */
+		if (ephy_session_present_window (session, url, user_time) == FALSE)
+		{
+			tab = ephy_shell_new_tab_full (shell, window,
+						       NULL /* parent tab */,
+						       url,
+						       flags | page_flags,
+						       EPHY_EMBED_CHROME_ALL,
+						       FALSE /* is popup? */,
+						       user_time);
 
-		window = EPHY_WINDOW (gtk_widget_get_toplevel (GTK_WIDGET (tab)));
+			window = EPHY_WINDOW (gtk_widget_get_toplevel (GTK_WIDGET (tab)));
+		}

Since we need the window also in the case of presenting the old one, I think you should make ephy_session_present_window return the window or NULL, not TRUE/FALSE.
If you open 2 URIs at the same time, and the 1st one has an existing window, the 2nd one will be opened in that window... I guess that's ok.

+    /* if address doesn't have web prefix, default to http:// */
+	if (!g_str_has_prefix (address, "http:") &&
+		!g_str_has_prefix (address, "https:") &&
+		!g_str_has_prefix (address, "ftp:") &&
+		!g_str_has_prefix (address, "file:") &&
+		!g_str_has_prefix (address, "data:") &&
+		!g_str_has_prefix (address, "about:") &&
+		!g_str_has_prefix (address, "gopher:"))
+
+	{
+		address = g_strconcat ("http://", address, NULL);
+	} 

Checking for "://" would be easier, and not hardcode that list in yet another place?

+			found = (strcmp (tab_address, address) == 0 ||
+				strcmp (tab_address, g_strconcat (address, "/", NULL)) == 0);

This leaks the result of the g_strconcat.
Also please add a "FIXME" comment about this being not the ideal comparision.

If we still want this I'll update the patch (if Tim is still alive he can do it ;)), it's a shame that it bitrots.

Yeah, I meant to get back to this but I've been so busy between work and school that I simply forgot about it. I really should've pinged back sooner. Sorry. :/

Feel free to update the patch.

Moving Severity:enhancement bugs off of 2.20 target.

I'm on this one, after EphyTab transition, a bit of cleanup has to be done.

I have a different use case, where I think, this issue applies (al least with epiphany 3.2):

Set:
- 2 epiphany windows.
- Each window lives in a different workspace.
- Each window has enough tabs, so new tabs are kept 'hidden' to the user.
- Using gnome-shell

Action:
- click on a url received in an email, chat, or a terminal

Problem:
- Where is the page opened?
- There is no feedback telling the use the url has been opened.

I have to go window by window, selecting the tabs to figure out which one opened the
page.

In gnome-shell, the activities does not help, because the url has been opened in
a new tab, which is hidden.

FWIW, in Chromium and Firefox, there is a notification saying something like:
'[App - url] is ready'. The user click on it, and it gets the application with the
url openend.

According to Diego, they might be using window_present or something like that
to raise the window, which is interpreted by the window manager.

I believe the problems raised by German are no longer a problem in current Epiphany. Epiphany presents the window after opening a new URI, so the behaviour should be similar to Chromium/Firefox. The point about raising an existing window/tab if the user requests that an already loaded URI be loaded stands, though. But I'm not sure I like the idea...

Closing this since it's a low-priority feature request, Gustavo doesn't like it, and I don't think I do either.