STEPS TO REPRODUCE: 1) Start Firefox with a clean profile. 2) Open a new tab. 3) Load http://web.mit.edu/bzbarsky/www/testcases/titles/no-title.html EXPECTED RESULTS: Tab title has the url in it. ACTUAL RESULTS: Tab title says "New Tab". This is a regression from bug 1364127; tested via local backout. There are other issues as well. For example, if I follow these steps: 1) Download http://web.mit.edu/bzbarsky/www/testcases/titles/no-title.html to /tmp 2) Start Firefox. 3) Open a new tab. 4) Load file:///tmp/no-title.html 5) Load http://web.mit.edu/bzbarsky/www/testcases/titles/no-title.html then at step 4 the tab title is "New Tab" but at step 5 the tab title is "file:///tmp/no-title.html", which is just _very_ wrong.

Note that once some page with a nonempty <title> title is loaded in a tab its tab title behaves reasonably after that.

[Tracking Requested - why for this release]: Tab titles with incorrect urls in them.

Tracking 55+ for this regression.

For comparison, here are the tab titles shown by other browsers for bz's no-title.html test case: - Firefox <= 54: "http://web.mit.edu/bzbarsky/www/testcases/titles/no-title.html" (full URL) - Chrome and Safari: "web.mit.edu/bzbarsky/www/testcases/titles/no-title.html" (URL without the http:// scheme) - IE and Edge: "web.mit.edu" Since Firefox shows the URL without the http:// scheme in the address bar, it seems to me that showing the URL without the scheme in the tab title (like Chrome and Safari) would make more sense than showing the full URL (like Firefox <= 54).

Dao, does this look good to land in the 55 window?

(In reply to Mike Taylor [:miketaylr] (55 Regression Engineering Owner) from comment #6) > Dao, does this look good to land in the 55 window? It might miss the nightly cycle but should still be fixed in beta.

Confirmed that this doesn't reproduce on 55b1. We should still track this for Nightly though.

OK, I just retested both on current trunk and 55b1, in a clean profile. Both show the bug for me with the both of the given sets of steps to reproduce. Which makes sense, since the code from bug 1364127 is not conditional in any way. I think comment 7 was about whether this fix will land or not, not about whether the regressing thing will land: that's landed. I should also note that this is being a daily productivity drain: I commonly have multiple file:// testcases all open at once as a normal part of my work, and this bug makes finding the one I need to deal with next incredibly painful. :(

I'm running on Win10 and tested again with Fx54, Fx55b1, and today's Nightly build, each with a fresh profile. I tested both STR from #c0. I get the same results for Fx54 and Fx55b1, the Expected Results. But on Nightly, I can still reproduce the same Actual Results bz reported. Not sure what explains that difference, though :\. Is it possible that the results depend on OS?

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10) > I'm running on Win10 and tested again with Fx54, Fx55b1, and today's Nightly > build, each with a fresh profile. I tested both STR from #c0. I get the same > results for Fx54 and Fx55b1, the Expected Results. But on Nightly, I can > still reproduce the same Actual Results bz reported. Not sure what explains > that difference, though :\. Is it possible that the results depend on OS? e10s might make a differences.

Tracking for 56 too. Hoping we get a fix here and can uplift to 55.

Ryan, can you reproduce w/ or w/o e10s like comment #11 suggests?

I have a fix for this bug I will post for Dao's review later today.

I think my patch has an intermittent test failure on non-e10s Linux that I need to investigate: browser/components/sessionstore/test/browser_tab_label_during_restore.js | observed tab label changes - Got 2, expected 1 In some cases the tab loading about:robots has two tab title changes ("" -> "New Tab" -> "Gort! Klaatu barada nikto!") instead of just one ("" -> "Gort! Klaatu barada nikto!"). I've seen a similar intermittent problem on my Mac when testing non-e10s mode. I think difference in e10s timing or IPC messages causes this problem to not happen on e10s. [task 2017-07-06T07:13:11.526657Z] 07:13:11 INFO - observing tab label changes. initial label: [task 2017-07-06T07:13:11.531657Z] 07:13:11 INFO - tab label change: New Tab [task 2017-07-06T07:13:11.536008Z] 07:13:11 INFO - tab label change: Gort! Klaatu barada nikto! https://treeherder.mozilla.org/#/jobs?repo=try&revision=bccf3f5acb41b7231c863e410ef5719e2458af38

I had a WIP patch for this and I'll need to check what parts of that still make sense with your changes. At the very least, I want to make browser_tab_label_during_restore.js more robust before fixing this. I'll file a new bug on that blocking this one.

(In reply to Dão Gottwald [::dao] from comment #21) > I had a WIP patch for this and I'll need to check what parts of that still > make sense with your changes. At the very least, I want to make > browser_tab_label_during_restore.js more robust before fixing this. I rebased my changes on top of your test changes, but my changes still cause a non-e10s only test failure. With e10s enabled, the about:robots tab title changes once: "" -> ABOUT_ROBOTS_TITLE observed tab label changes - Got ["","Gort! Klaatu barada nikto!"] With e10s disabled, the about:robots tab title changes twice: "" -> "New Tab" -> ABOUT_ROBOTS_TITLE. observed tab label changes - Got ["","New Tab","Gort! Klaatu barada nikto!"], expected ["","Gort! Klaatu barada nikto!"]

Since today, when I open a XPI in a new tab, it don't install before I reload the page. XPI to test can be: https://ulqa.avira.com/package/abs/firefox/abs-webext-beta.xpi Till yesterday I got a "New Tab"-page, but it worked.

Dão, I've removed my review requests because there is still a bug in my patches that I have figured out yet. If your WIP works, feel free to land it if I haven't solved this bug soon. For some reason I haven't figured out yet, when the test selects the second tab, two DOMTitleChanged events are fired: * When e10s is disabled, a DOMTitleChanged event for about:blank and then one for about:robots * When e10s is enabled, two DOMTitleChanged events for about:robots

I also had trouble fixing this without regressing other things, but I'll see if I can finish my patch.

(In reply to Dão Gottwald [::dao] from comment #25) > I also had trouble fixing this without regressing other things, but I'll see > if I can finish my patch. Ahem, somehow I seem to have fixed this before I put my WIP patch aside. Just applied it again and it does seem to fix this bug without regressing tests. Maybe I'm missing something.

Comment on attachment 8885664 [details] [diff] [review] patch? Review of attachment 8885664 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Some minor issues I found testing your patch: 1. Your patch's regular expression strips all URL protocols including "file://", not just "http://" and "https://". When I load a local HTML file, Firefox currently shows the file: URL in the tab title (e.g. "file:///Users/chris/Desktop/test.html"), but your patch shows the file path in the tab title like "/Users/chris/Desktop/test.html". I think your patch's tab titles are nicer, but I just wanted to make sure this change was intended. 2. When browsing from "/Users/chris/Desktop/test.html" to a remote web page, the real URL "file:///Users/chris/Desktop/test.html" briefly flashes in the tab title before switching to the new web page's title. In Firefox 54, this is when the "Connecting..." string would be shown in the tab title.

(In reply to Chris Peterson [:cpeterson] from comment #27) > Comment on attachment 8885664 [details] [diff] [review] > patch? > > Review of attachment 8885664 [details] [diff] [review]: > ----------------------------------------------------------------- > > LGTM. Some minor issues I found testing your patch: > > 1. Your patch's regular expression strips all URL protocols including > "file://", not just "http://" and "https://". When I load a local HTML file, > Firefox currently shows the file: URL in the tab title (e.g. > "file:///Users/chris/Desktop/test.html"), but your patch shows the file path > in the tab title like "/Users/chris/Desktop/test.html". I think your patch's > tab titles are nicer, but I just wanted to make sure this change was > intended. Yeah, this was intended. > 2. When browsing from "/Users/chris/Desktop/test.html" to a remote web page, > the real URL "file:///Users/chris/Desktop/test.html" briefly flashes in the > tab title before switching to the new web page's title. In Firefox 54, this > is when the "Connecting..." string would be shown in the tab title. I think this is because we flip the remoteness of that browser in that case, and for that we utilize sessionstore, and sessionstore stores the label as the page's title regardless of whether it's a content title or the URL, and then we get it back not knowing that it's a URL: http://searchfox.org/mozilla-central/rev/cef8389c687203085dc6b52de2fbd0260d7495bf/browser/components/sessionstore/SessionStore.jsm#2647-2648

Comment on attachment 8886499 [details] Bug 1367630 - part 1: Refactor tab label update mechanism to better handle tabs without a content title. f=cpeterson https://reviewboard.mozilla.org/r/157304/#review162916 This looks good Dão! I am curious about one change though. Can you answer that for me? ::: browser/base/content/tabbrowser.xml:1556 (Diff revision 1) > + > + aOptions = aOptions || {}; > + if (!aOptions.isContentTitle) { > + // Remove protocol and "www." > + if (!("_regex_shortenURLForTabLabel" in this)) { > + this._regex_shortenURLForTabLabel = /^[^:]+:\/\/(?:www\.)?/; Out of curiosity, why did you choose to use a regex here and not the URIFixup code as removed above?

Comment on attachment 8886500 [details] Bug 1367630 - part 2: Rename _suppressTransientPlaceholderLabel to _labelIsInitialTitle. f=cpeterson https://reviewboard.mozilla.org/r/157306/#review162918 Yup, much clearer :)

Comment on attachment 8886501 [details] Bug 1367630 - part 3: Show the full label rather than the truncated one in tab tooltips. https://reviewboard.mozilla.org/r/157308/#review162920 This looks very good to me, but I'm curious about one thing here too: ::: browser/base/content/tabbrowser.xml:1552 (Diff revision 1) > <![CDATA[ > if (!aLabel) { > return false; > } > > + aTab._fullLabel = aLabel; Why not say `aTab.setAttribute("fullLabel", aLabel);`? I think that'd make this more generally available and perhaps might be useful to persist for sessionstore as a TabAttribute?

Comment on attachment 8886502 [details] Bug 1367630 - part 4: Don't claim that the stored title is a content title when it's really the page's URL. https://reviewboard.mozilla.org/r/157310/#review162926 Agreed :)

(In reply to Mike de Boer [:mikedeboer] from comment #34) > ::: browser/base/content/tabbrowser.xml:1556 > (Diff revision 1) > > + > > + aOptions = aOptions || {}; > > + if (!aOptions.isContentTitle) { > > + // Remove protocol and "www." > > + if (!("_regex_shortenURLForTabLabel" in this)) { > > + this._regex_shortenURLForTabLabel = /^[^:]+:\/\/(?:www\.)?/; > > Out of curiosity, why did you choose to use a regex here and not the > URIFixup code as removed above? It was used to get the host. I'm not interested in the host here since I only truncate from the front. (In reply to Mike de Boer [:mikedeboer] from comment #36) > ::: browser/base/content/tabbrowser.xml:1552 > (Diff revision 1) > > <![CDATA[ > > if (!aLabel) { > > return false; > > } > > > > + aTab._fullLabel = aLabel; > > Why not say `aTab.setAttribute("fullLabel", aLabel);`? I think that'd make > this more generally available and I made it private because my only use is private. If there's a need beyond that, somebody will have to make a case for that. > perhaps might be useful to persist for > sessionstore as a TabAttribute? To what end?

(In reply to Dão Gottwald [::dao] from comment #39) > It was used to get the host. I'm not interested in the host here since I > only truncate from the front. > > I made it private because my only use is private. If there's a need beyond > that, somebody will have to make a case for that. Agreed.

Comment on attachment 8886499 [details] Bug 1367630 - part 1: Refactor tab label update mechanism to better handle tabs without a content title. f=cpeterson https://reviewboard.mozilla.org/r/157304/#review163518 Ship it!

Comment on attachment 8886501 [details] Bug 1367630 - part 3: Show the full label rather than the truncated one in tab tooltips. https://reviewboard.mozilla.org/r/157308/#review163522

Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/32af7843b1cb part 1: Refactor tab label update mechanism to better handle tabs without a content title. f=cpeterson r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/deab0ebdcd7c part 2: Rename _suppressTransientPlaceholderLabel to _labelIsInitialTitle. f=cpeterson r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/d8b3bb26abe0 part 3: Show the full label rather than the truncated one in tab tooltips. r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/e8e5ca57b442 part 4: Don't claim that the stored title is a content title when it's really the page's URL. r=mikedeboer

https://hg.mozilla.org/mozilla-central/rev/32af7843b1cb https://hg.mozilla.org/mozilla-central/rev/deab0ebdcd7c https://hg.mozilla.org/mozilla-central/rev/d8b3bb26abe0 https://hg.mozilla.org/mozilla-central/rev/e8e5ca57b442

Comment on attachment 8886499 [details] Bug 1367630 - part 1: Refactor tab label update mechanism to better handle tabs without a content title. f=cpeterson Approval Request Comment [Feature/Bug causing the regression]: bug 1364127 [User impact if declined]: see comment 0 [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: yes, see comment 0 [List of other uplifts needed for the feature/fix]: / [Is the change risky?]: Somewhat risky, but we should still take it once verified. Regressions aren't unlikely, but it's unlikely that they would be worse than this bug. [Why is the change risky/not risky?]: We had some trouble coming up with a fix that doesn't break other things, since different pieces of code are involved and interact in sometimes obscure ways. [String changes made/needed]: / Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b591ec18b518383ea6304e5794e99f62ed0e128c

there is a slight talos regression in the damp (devtools) test: == Change summary for alert #8093 (as of July 20 2017 06:04 UTC) == Regressions: 2% damp summary linux64 opt e10s 236.85 -> 240.82 1% damp summary windows7-32 opt e10s 257.94 -> 260.61 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8093 this is technically below the 2% threshold that we file new bugs for, but it was mixed in near other regressions and we wanted to separate it from the other changes.

(In reply to Joel Maher ( :jmaher) (UTC-8) from comment #46) > there is a slight talos regression in the damp (devtools) test: > == Change summary for alert #8093 (as of July 20 2017 06:04 UTC) == > > Regressions: > > 2% damp summary linux64 opt e10s 236.85 -> 240.82 > 1% damp summary windows7-32 opt e10s 257.94 -> 260.61 I expect this can be ignored. We can't cater for damp for general UI changes like this one that should affect devtools performance only very indirectly.

I have reproduced this bug with Nightly 55.0a1 (2017-05-24) on Windows 8.1 , 64 Bit ! This bug's fix is Verified with latest Nightly 56.0a1 ! Build ID 20170721030204 User Agent Mozilla/5.0 (Windows NT 6.3; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0

When opening pages that load slowly, e.g. Facebook, I'm seeing the tab title go from "New Tab" to "facebook.com/" to "Facebook". I think I wasn't seeing this a few days ago (the transition was "New Tab" -> "Facebook"). Could the patches in this bug have caused this regression?

I can consistently reproduce by opening "https://hg.mozilla.org/mozilla-central/rev/32af7843b1cb". mozregression said: > 16:01.96 INFO: Last good revision: 2fb0f93b7b5bc9880f57534673f0e27c6b5a789e > 16:01.96 INFO: First bad revision: e8e5ca57b442fd63555e5e17ddcf96a339a0a6d1 > 16:01.97 INFO: Pushlog: > https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2fb0f93b7b5bc9880f57534673f0e27c6b5a789e&tochange=e8e5ca57b442fd63555e5e17ddcf96a339a0a6d1 So yes, the regression is coming from these patches.

(In reply to Marco Castelluccio [:marco] from comment #49) > When opening pages that load slowly, e.g. Facebook, I'm seeing the tab title > go from "New Tab" to "facebook.com/" to "Facebook". I think I wasn't seeing > this a few days ago (the transition was "New Tab" -> "Facebook"). That's a feature, not a bug. Ideally we shouldn't show "New Tab" at all, that's basically bug 610357.

(In reply to Dão Gottwald [::dao] from comment #51) > (In reply to Marco Castelluccio [:marco] from comment #49) > > When opening pages that load slowly, e.g. Facebook, I'm seeing the tab title > > go from "New Tab" to "facebook.com/" to "Facebook". I think I wasn't seeing > > this a few days ago (the transition was "New Tab" -> "Facebook"). > > That's a feature, not a bug. Ideally we shouldn't show "New Tab" at all, > that's basically bug 610357. This was not session restore. I was on a new tab, so "New Tab" was simply the title of the page.

(In reply to Marco Castelluccio [:marco] from comment #52) > (In reply to Dão Gottwald [::dao] from comment #51) > > (In reply to Marco Castelluccio [:marco] from comment #49) > > > When opening pages that load slowly, e.g. Facebook, I'm seeing the tab title > > > go from "New Tab" to "facebook.com/" to "Facebook". I think I wasn't seeing > > > this a few days ago (the transition was "New Tab" -> "Facebook"). > > > > That's a feature, not a bug. Ideally we shouldn't show "New Tab" at all, > > that's basically bug 610357. > > This was not session restore. I was on a new tab, so "New Tab" was simply > the title of the page. I realize that. Bug 610357 is not a session restore bug.

jcristau, can we get this approved? Any further delay here is counterproductive as it reduces the baking time on 56 and increases the risk of missing potential regressions. FWIW, the two regression bugs filed so far, bug 1383873 and bug 1383637, should not prevent the uplift.

Comment on attachment 8886499 [details] Bug 1367630 - part 1: Refactor tab label update mechanism to better handle tabs without a content title. f=cpeterson let's get this patch set in 55.0b13 Thanks Dão for comment 54.

https://hg.mozilla.org/releases/mozilla-beta/rev/cd24a521812a8d82a73e7b91c781a2f3fdeeb141 https://hg.mozilla.org/releases/mozilla-beta/rev/1aa87709eb566fe1b64f2fd5d2466ecfc1b7e699 https://hg.mozilla.org/releases/mozilla-beta/rev/5d60a973693b73e39d1780fe6e70d531c675635f https://hg.mozilla.org/releases/mozilla-beta/rev/116c714b341eecf34d466907930ad2629f7d9b6a

I have reproduced this issue using Firefox 55.0a1 (2017.05.24) on Win 8.1 x64. I can confirm this issue is fixed, I verified using Firefox 56.0a1, on Win 8.1 x64, Mac OS X 10.10 and Ubuntu 14.04 x64. I will verify on 55.0b13!

I can confirm this issue is fixed, I verified using Firefox 55.0b13, on Win 8.1 x64, Mac OS X 10.10 and Ubuntu 14.04 x64.