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 774599 - Pressing Ctrl+T to open a new tab does not focus the location bar entry
Pressing Ctrl+T to open a new tab does not focus the location bar entry
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Interface
git master
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-11-16 22:39 UTC by Adrian Perez
Modified: 2016-12-05 14:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ephy-web-view: Always update the internal address when WebKitWebView:uri changes (3.33 KB, patch)
2016-12-03 12:36 UTC, Carlos Garcia Campos
committed Details | Review
ephy-embed-utils: overview and incognito URLs should also be considered as empty (1.79 KB, patch)
2016-12-03 12:37 UTC, Carlos Garcia Campos
committed Details | Review
ephy-window: about:blank and incognito pages should also be considered as blank (2.15 KB, patch)
2016-12-03 12:37 UTC, Carlos Garcia Campos
committed Details | Review
prefs: Remove homepage-loads-in-new-tabs setting (2.98 KB, patch)
2016-12-03 12:38 UTC, Carlos Garcia Campos
committed Details | Review
window: focus the location entry when opening new tabs (3.76 KB, patch)
2016-12-03 12:39 UTC, Carlos Garcia Campos
committed Details | Review

Description Adrian Perez 2016-11-16 22:39:41 UTC
Steps to reproduce
==================

1. Obtain the current Git “master” (commit 7688c7d).
2. Build Epiphany and run it.
3. Press Ctrl+T to open a new tab.

Expected outcome
----------------
A new tab is opened, and the location bar entry is focused.

Actual outcome
--------------
A new tab is opened, and the webview is focused.
Comment 1 Michael Catanzaro 2016-11-16 22:48:14 UTC
A bisect would be really helpful here!
Comment 2 Carlos Garcia Campos 2016-12-03 12:29:25 UTC
This is irritating, I always end up writing for nothing after CTRL + T
Comment 3 Carlos Garcia Campos 2016-12-03 12:36:26 UTC
Created attachment 341305 [details] [review]
ephy-web-view: Always update the internal address when WebKitWebView:uri changes
Comment 4 Carlos Garcia Campos 2016-12-03 12:37:03 UTC
Created attachment 341306 [details] [review]
ephy-embed-utils: overview and incognito URLs should also be considered as empty
Comment 5 Carlos Garcia Campos 2016-12-03 12:37:42 UTC
Created attachment 341307 [details] [review]
ephy-window: about:blank and incognito pages should also be considered as blank
Comment 6 Carlos Garcia Campos 2016-12-03 12:38:21 UTC
Created attachment 341308 [details] [review]
prefs: Remove homepage-loads-in-new-tabs setting
Comment 7 Carlos Garcia Campos 2016-12-03 12:39:05 UTC
Created attachment 341309 [details] [review]
window: focus the location entry when opening new tabs
Comment 8 Michael Catanzaro 2016-12-03 16:32:35 UTC
Review of attachment 341305 [details] [review]:

OK
Comment 9 Michael Catanzaro 2016-12-03 16:34:09 UTC
Review of attachment 341306 [details] [review]:

Why do we have to test for ephy-about: and not about: like we do for about:blank? It's strange that we have to check different schemes here when they're all defined by Epiphany, right?
Comment 10 Michael Catanzaro 2016-12-03 16:34:53 UTC
Review of attachment 341307 [details] [review]:

OK
Comment 11 Michael Catanzaro 2016-12-03 16:39:22 UTC
Review of attachment 341308 [details] [review]:

The problem is that different users really want different behavior here:

 * If you're using Ephy as a desktop browser and for some reason you really want a homepage, you probably do not want it to load in every new tab, that wastes a lot of time. This is the more common case, so if removing the setting I would make it not load in new tabs at all.
 * An exception is some web kiosk environments, where it really is desired that users always get the homepage in new tabs. This was yet another one of those "I would love to use Epiphany if only..." style requests that I decided to eliminate as an excuse for not using Epiphany.

So I prefer to keep the setting, but if you're really set on removing it, you should at least pick the opposite behavior.
Comment 12 Michael Catanzaro 2016-12-03 16:58:06 UTC
Review of attachment 341309 [details] [review]:

Nice cleanup!

In the commit message: llogic -> logic

::: src/ephy-window.c
@@ +324,1 @@
+  g_assert (address != NULL || flags & EPHY_LINK_HOME_PAGE);

To keep the setting, just allow address to be NULL if EPHY_LINK_NEW_TAB is passed...

@@ +365,3 @@
+  if (flags & EPHY_LINK_HOME_PAGE)
+    ephy_web_view_load_homepage (web_view);
+  else

...and load about:overview (or about:blank) if flags & EPHY_LINK_NEW_TAB here...

@@ +369,3 @@
+
+  if (ephy_web_view_get_is_blank (web_view))
+    ephy_window_activate_location (window);

(This check still works either way! I think it's totally fair to not focus the location entry if you have decided to set a homepage.)

::: src/window-commands.c
@@ -869,3 @@
-  gboolean load_homepage;
-
-  load_homepage = g_settings_get_boolean (EPHY_SETTINGS_MAIN,

...then don't remove the setting here...

@@ -876,2 @@
   ephy_link_open (EPHY_LINK (window),
-                  "about:overview", /* Ignored when EPHY_LINK_HOME_PAGE is passed */

...but if you do all of that, you can still get rid of the "about:overview" from here.
Comment 13 Carlos Garcia Campos 2016-12-04 08:09:09 UTC
(In reply to Michael Catanzaro from comment #9)
> Review of attachment 341306 [details] [review] [review]:
> 
> Why do we have to test for ephy-about: and not about: like we do for
> about:blank? It's strange that we have to check different schemes here when
> they're all defined by Epiphany, right?

No, about:blank is defined by WebKit, it's a special case, that why we can't register about and we have to register ephy-about instead to implement our about pages. about: is not a custom URI scheme, ephy-about: is.
Comment 14 Carlos Garcia Campos 2016-12-04 08:32:03 UTC
(In reply to Michael Catanzaro from comment #11)
> Review of attachment 341308 [details] [review] [review]:
> 
> The problem is that different users really want different behavior here:
> 
>  * If you're using Ephy as a desktop browser and for some reason you really
> want a homepage, you probably do not want it to load in every new tab, that
> wastes a lot of time. This is the more common case, so if removing the
> setting I would make it not load in new tabs at all.

Ah, I see.

>  * An exception is some web kiosk environments, where it really is desired
> that users always get the homepage in new tabs. This was yet another one of
> those "I would love to use Epiphany if only..." style requests that I
> decided to eliminate as an excuse for not using Epiphany.

There are no tabs in kiosk mode. Could you point me to where this feature has been requested? is there a bug or mail?

> So I prefer to keep the setting, but if you're really set on removing it,
> you should at least pick the opposite behavior.

It took me a while to understand the setting to be honest. I've checked what both firefox and chromium do.

 - Chromium: you can only set a homepage for the home button, that is only shown in the toolbar if you set a homepage. That homepage is never used for new tbas nor new windows, only when you press the button. There's another setting to specify a set of pages to load at startup.

 - Firefox: you can set a homepage that is used that is used when pressing the home button as expected. It can also be used at start up and for new windows if you set another setting to load homepage at startup. Never used for new tabs.

So, yes, in both cases the homepage is never loaded on new tabs. If we are adding this for users I don't think they will be happy to have it so hidden, it's not exposed in the preferences dialog and we don't have a home button (I had to check the source code to figure out ALT+Home loads the home page).

So, now that I understand it, I propose to keep only the homepage setting, but exposed in the preferences dialog, and with a mixed behavior between firefox and chrome. We add a setting to set the homepage to the general tab. Only when we have a homepage set, a home button is shown in the toolbar. We can keep the single setting for the startup, when not loading session we use the homepage is set or the overview otherwise. And of course we never load the homepage on new tabs, only on new windows (including the new window at startup when not loading the session).

What do you think?
Comment 15 Michael Catanzaro 2016-12-04 16:19:31 UTC
(In reply to Carlos Garcia Campos from comment #14)
> There are no tabs in kiosk mode. Could you point me to where this feature
> has been requested? is there a bug or mail?

This was someone on Reddit who evidently wanted to keep the window chrome, I really don't know why. Since it looks like this behavior isn't possible in other browsers and it is pretty strange, that argues that we don't need to support it.
 
> So, now that I understand it, I propose to keep only the homepage setting,
> but exposed in the preferences dialog, and with a mixed behavior between
> firefox and chrome. We add a setting to set the homepage to the general tab.
> Only when we have a homepage set, a home button is shown in the toolbar. We
> can keep the single setting for the startup, when not loading session we use
> the homepage is set or the overview otherwise. And of course we never load
> the homepage on new tabs, only on new windows (including the new window at
> startup when not loading the session).
> 
> What do you think?

OK, that sounds good to me.

Perhaps the homepage setting should only be available if "Restore previous tabs on startup" is unchecked?
Comment 16 Michael Catanzaro 2016-12-04 20:20:36 UTC
Review of attachment 341306 [details] [review]:

::: embed/ephy-embed-utils.c
@@ +269,3 @@
+          location[0] == '\0' ||
+          strcmp (location, "about:blank") == 0 ||
+          strcmp (location, "ephy-about:overview") == 0 ||

Actually I think we should add plain "about:overview" to this list as well. I just noticed a bug today where, if the overview is open on the current tab when you close Ephy, next time you open Ephy "about:overview" will appear in the location entry for a fraction of a second. Maybe it would fix that?
Comment 17 Michael Catanzaro 2016-12-04 20:21:13 UTC
(Not tested, nor checked to see if that's right... don't add it blindly.)
Comment 18 Carlos Garcia Campos 2016-12-05 10:40:35 UTC
(In reply to Michael Catanzaro from comment #16)
> Review of attachment 341306 [details] [review] [review]:
> 
> ::: embed/ephy-embed-utils.c
> @@ +269,3 @@
> +          location[0] == '\0' ||
> +          strcmp (location, "about:blank") == 0 ||
> +          strcmp (location, "ephy-about:overview") == 0 ||
> 
> Actually I think we should add plain "about:overview" to this list as well.
> I just noticed a bug today where, if the overview is open on the current tab
> when you close Ephy, next time you open Ephy "about:overview" will appear in
> the location entry for a fraction of a second. Maybe it would fix that?

No, that's a different bug. ephy_embed_utils_url_is_empty() as well as ephy_embed_utils_is_no_show_address() should always be used with the effective url, not the typed url. And that's how they are currently used.
Comment 19 Carlos Garcia Campos 2016-12-05 11:59:37 UTC
(In reply to Michael Catanzaro from comment #15)
> (In reply to Carlos Garcia Campos from comment #14)
> > There are no tabs in kiosk mode. Could you point me to where this feature
> > has been requested? is there a bug or mail?
> 
> This was someone on Reddit who evidently wanted to keep the window chrome, I
> really don't know why. Since it looks like this behavior isn't possible in
> other browsers and it is pretty strange, that argues that we don't need to
> support it.
>  
> > So, now that I understand it, I propose to keep only the homepage setting,
> > but exposed in the preferences dialog, and with a mixed behavior between
> > firefox and chrome. We add a setting to set the homepage to the general tab.
> > Only when we have a homepage set, a home button is shown in the toolbar. We
> > can keep the single setting for the startup, when not loading session we use
> > the homepage is set or the overview otherwise. And of course we never load
> > the homepage on new tabs, only on new windows (including the new window at
> > startup when not loading the session).
> > 
> > What do you think?
> 
> OK, that sounds good to me.
> 
> Perhaps the homepage setting should only be available if "Restore previous
> tabs on startup" is unchecked?

No, you can set a homepage, but still restore session, it will be used when you click the home button, or when you open a new window.
Comment 20 Carlos Garcia Campos 2016-12-05 14:17:41 UTC
Comment on attachment 341309 [details] [review]
window: focus the location entry when opening new tabs

Updated and committed