Closed Bug 1359352 Opened 3 years ago Closed 3 years ago

Don't show »Connecting« and »Loading« labels in tab while loading pages

Categories

(Firefox :: Tabbed Browser, enhancement, P1)

All
Unspecified
enhancement

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.5 - May 15
Tracking Status
firefox55 --- fixed

People

(Reporter: phlsa, Assigned: past)

References

Details

(Whiteboard: [photon-performance])

Attachments

(1 file)

When navigating from one page to another, Firefox replaces the title of the old page in the tab with »Connecting« and »Loading«. Especially when load times are fast, this makes Firefox feel quite a bit more »nervous« than other browsers that don't do that.

We should instead keep whatever string has previously been in the tab (so normally the title of the page that is being navigated away from) and show the loading indicator next to that.

The one exception I would not here is when opening a link in a new tab. In that case, I would suggest using either »New Tab« or »Loading« as the label until the title of the page that gets loaded is available.
Flags: qe-verify+
Whiteboard: [photon-performance]
Assignee: nobody → past
Status: NEW → ASSIGNED
Comment on attachment 8864856 [details]
Don't show 'Connecting' and 'Loading' labels in tab while loading pages (bug 1359352).

https://reviewboard.mozilla.org/r/136550/#review139654

::: browser/base/content/tabbrowser.xml:1409
(Diff revision 1)
>          ]]></body>
>        </method>
>  
>  
> +      <!-- TODO: remove after 57, once we know add-ons can no longer use it. -->
>        <method name="setTabTitleLoading">

So you're just keeping this around so that add-on callers don't fail ungracefully? Why not make this method a no-op then?
(In reply to Dão Gottwald [::dao] from comment #2)
> So you're just keeping this around so that add-on callers don't fail
> ungracefully? Why not make this method a no-op then?

I thought there might be some cases where the add-on intentionally displays this title, but I guess there are other ways to do that.
(In reply to (Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #0)
> The one exception I would not here is when opening a link in a new tab. In
> that case, I would suggest using either »New Tab« or »Loading« as the label
> until the title of the page that gets loaded is available.

The current patch doesn't quite do this. New tabs opened from links get no label until the page title arrives.

You'll also need to fix or remove this:
http://searchfox.org/mozilla-central/rev/6580dd9a4eb0224773897388dba2ddf5ed7f4216/browser/base/content/tabbrowser.xml#739-740
Other than the issue that Dão mentioned, this looks good to me!
(In reply to Dão Gottwald [::dao] from comment #5)
> (In reply to (Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX)
> please use needinfo from comment #0)
> > The one exception I would not here is when opening a link in a new tab. In
> > that case, I would suggest using either »New Tab« or »Loading« as the label
> > until the title of the page that gets loaded is available.
> 
> The current patch doesn't quite do this. New tabs opened from links get no
> label until the page title arrives.
> 
> You'll also need to fix or remove this:
> http://searchfox.org/mozilla-central/rev/
> 6580dd9a4eb0224773897388dba2ddf5ed7f4216/browser/base/content/tabbrowser.
> xml#739-740

Thanks for spotting both issues, they should be fixed in the last patch.
Comment on attachment 8864856 [details]
Don't show 'Connecting' and 'Loading' labels in tab while loading pages (bug 1359352).

https://reviewboard.mozilla.org/r/136550/#review139678

::: browser/base/content/tabbrowser.xml:694
(Diff revision 2)
>                  }
>  
>                  if (this._shouldShowProgress(aRequest)) {
>                    if (!(aStateFlags & nsIWebProgressListener.STATE_RESTORING)) {
>                      this.mTab.setAttribute("busy", "true");
> -
> +                    this.mTabBrowser.setTabTitle(this.mTab);

I'm not sure this is working as intended. I still see the empty label when open links in new tabs.

::: browser/base/content/tabbrowser.xml
(Diff revision 2)
> +      <!-- TODO: remove after 57, once we know add-ons can no longer use it. -->
>        <method name="setTabTitleLoading">
>          <parameter name="aTab"/>
>          <body>
>            <![CDATA[
> -            aTab.label = this.mStringBundle.getString("tabs.connecting");

Please remove this string from tabbrowser.properties.

To do that, though, you'll also need to do something about SessionStore.jsm's _replaceLoadingTitle.

::: browser/base/content/tabbrowser.xml:1411
(Diff revision 2)
>          <parameter name="aTab"/>
>          <body>
>            <![CDATA[
> -            aTab.label = this.mStringBundle.getString("tabs.connecting");
> -            this._tabAttrModified(aTab, ["label"]);
>            ]]>

You can remove <![CDATA[ ]]>. Just <body/> probably works too.
Attachment #8864856 - Flags: review?(dao+bmo) → review-
(In reply to Dão Gottwald [::dao] from comment #10)
> I'm not sure this is working as intended. I still see the empty label when
> open links in new tabs.

I can't reproduce this. Are you using a local build or the try push I posted earlier? The latter doesn't have the fix. Here is a new one:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3a63c41e247f7f21ffcdac1a6545f944bb89c1e

> > -            aTab.label = this.mStringBundle.getString("tabs.connecting");
> 
> Please remove this string from tabbrowser.properties.

tabs.connecting is the only string still used in this group, but we haven't removed the rest either:

tabs.connecting=Connecting…
tabs.encryptingConnection=Securing connection…
tabs.searching=Searching…
tabs.loading=Loading…
tabs.waiting=Waiting…
tabs.downloading=Downloading…

Shall I remove them all?
(In reply to Panos Astithas [:past] (please needinfo?) from comment #11)
> (In reply to Dão Gottwald [::dao] from comment #10)
> > I'm not sure this is working as intended. I still see the empty label when
> > open links in new tabs.
> 
> I can't reproduce this. Are you using a local build or the try push I posted
> earlier? The latter doesn't have the fix. Here is a new one:
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=b3a63c41e247f7f21ffcdac1a6545f944bb89c1e

I was using a local build.

> > > -            aTab.label = this.mStringBundle.getString("tabs.connecting");
> > 
> > Please remove this string from tabbrowser.properties.
> 
> tabs.connecting is the only string still used in this group, but we haven't
> removed the rest either:
> 
> tabs.connecting=Connecting…
> tabs.encryptingConnection=Securing connection…
> tabs.searching=Searching…
> tabs.loading=Loading…
> tabs.waiting=Waiting…
> tabs.downloading=Downloading…
> 
> Shall I remove them all?

Yes, this is cruft from bug 603717. These strings weren't ever used.
(In reply to Dão Gottwald [::dao] from comment #12)
> Yes, this is cruft from bug 603717. These strings weren't ever used.

I see some usage still in add-ons though:
https://dxr.mozilla.org/addons/search?q=tabs.connecting&redirect=true
Comment on attachment 8864856 [details]
Don't show 'Connecting' and 'Loading' labels in tab while loading pages (bug 1359352).

https://reviewboard.mozilla.org/r/136550/#review140080

(In reply to Dão Gottwald [::dao] from comment #10)
> I still see the empty label when open links in new tabs.

Still seeing it. There's a split second where the tab is blank before changing to "New Tab" (and then the page title, and I think sometimes the URL before the title arrives?).

Although even if the above is fixed, using "New Tab", then the URL, then the title still feels "nervous." Perhaps we should use the URL from the start rather than New Tab?
Attachment #8864856 - Flags: review?(dao+bmo) → review-
See Also: → 1362774
Priority: -- → P1
QA Contact: adrian.florinescu
Comment on attachment 8864856 [details]
Don't show 'Connecting' and 'Loading' labels in tab while loading pages (bug 1359352).

https://reviewboard.mozilla.org/r/136550/#review140080

We discussed this and I got some screen recordings which showed that the behavior here is unchanged with the patch. The other point about using the URL from the beginning instead of New Tab is very hard: the URL known at that point is about:blank, which doesn't buy us anything, and making sure that the URL is passed on correctly is likely the same as bug 610357, which is a pretty hard bug to fix.

So this patch significantly improves the perceived performance on same-tab navigation and is a minor improvement in the new tab navigation. I think that is all we can hope for at the moment.
Comment on attachment 8864856 [details]
Don't show 'Connecting' and 'Loading' labels in tab while loading pages (bug 1359352).

https://reviewboard.mozilla.org/r/136550/#review141074
Attachment #8864856 - Flags: review?(dao+bmo) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7cb81c02e439
Don't show 'Connecting' and 'Loading' labels in tab while loading pages . r=dao
(In reply to Panos Astithas [:past] (please needinfo?) from comment #16)
> The other point about using the
> URL from the beginning instead of New Tab is very hard: the URL known at
> that point is about:blank

Could this be done for about:home in a new window (bug 1362774)?
(In reply to Marco Castelluccio [:marco] from comment #21)
> Could this be done for about:home in a new window (bug 1362774)?

I don't think this is special-cased in a way conducive to a case-specific workaround, so comment 16 applies there as well.
https://hg.mozilla.org/mozilla-central/rev/7cb81c02e439
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Iteration: --- → 55.5 - May 15
One thing I've noticed is that if you use session restore and there are tabs in background that contain media - for example an youtube, then when focusing and resuming it, the backround tab will transition from the _title to new tab and then back to _title.
That is expected, previously it would transition from title to "Connecting..." to title again. Not going through about:blank (which is what this is) is a much harder problem to solve.
See Also: → 1364127
(In reply to Adrian Florinescu [:AdrianSV] from comment #24)
> One thing I've noticed is that if you use session restore and there are tabs
> in background that contain media - for example an youtube, then when
> focusing and resuming it, the backround tab will transition from the _title
> to new tab and then back to _title.

I filed bug 1364127 about this.
Depends on: 1364127
See Also: 1364127
Depends on: 1365448
No longer depends on: 1365448
I have reproduced this bug with Nightly 55.0a1 (2017-04-25) on Windows 8.1 (64 bit).

This bug's fix is verified on Latest Nightly 55.0a1.

Build ID  : 20170517030204
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday 20170517]
Thank you.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
I frequently open links in new background tabs and have been experiencing this new feature for several days now, so I frequently see the page's URL in the tab. My tabs are maxed out on width so in practice this means I mostly see https://www a lot - or perhaps (for a wider tab) https://bugzilla.m 

I find this behavior useless and distracting. It's extra information that's unneeded and ugly; only there for a short period so I feel like I may have missed some information I was supposed to read.  If this string were static, like "Loading..." it wouldn't be an extra mental burden, but as is feels like extra noise.

Please consider replacing the URL with a static string to reduce the cognitive burden this creates.
(In reply to Caspy7 from comment #29)
> I frequently open links in new background tabs and have been experiencing
> this new feature for several days now, so I frequently see the page's URL in
> the tab. My tabs are maxed out on width so in practice this means I mostly
> see https://www a lot - or perhaps (for a wider tab) https://bugzilla.m 
> 
> I find this behavior useless and distracting. It's extra information that's
> unneeded and ugly; only there for a short period so I feel like I may have
> missed some information I was supposed to read.  If this string were static,
> like "Loading..." it wouldn't be an extra mental burden, but as is feels
> like extra noise.
> 
> Please consider replacing the URL with a static string to reduce the
> cognitive burden this creates.

I think this comment is really directed to Philipp, so adding a needinfo to ensure he sees it.
Flags: needinfo?(philipp)
(In reply to Caspy7 from comment #29)
> I frequently open links in new background tabs and have been experiencing
> this new feature for several days now, so I frequently see the page's URL in
> the tab. My tabs are maxed out on width so in practice this means I mostly
> see https://www a lot - or perhaps (for a wider tab) https://bugzilla.m 

I came here to make a similar suggestion: instead of showing the complete URL (e.g. "https://en.m.wikipedia.org/wiki/Main_Page"), maybe show something less technical and more user-friendly like:

- the URL without the URL scheme: "en.m.wikipedia.org/wiki/Main_Page"
- just the domain name: "en.m.wikipedia.org"
- or just the TLD+1: "wikipedia.org"

My preference is for showing just the TLD+1. The user thinks "I am opening a Wikipedia tab" and then sees a tab titled "wikipedia.org". With this approach, you avoid avoid tab titles with "www." prefixes (which are just as unhelpful as tab titles like "https://...") without special heuristics for deciding which subdomain names are meaningful for users or not.
I actually filed bug 1366870 a couple days ago to strip out http:// and "www." from URLs in the title, but did not want to distract from my suggestion to keep the text static. Because I don't think the URL adds sufficient benefit over the added noise (as I went on about previously).
Depends on: 1366870
I agree that this is not a good experience. Let's continue the discussion in bug 1366870.
Flags: needinfo?(philipp)
You need to log in before you can comment on or make changes to this bug.