Brindusa Tot, Desktop QA Reporter
	Description • 9 years ago

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:45.0) Gecko/20100101 Firefox/45.0
Nightly 44 build from 11.29.2015 migrated to Nightly 45 buildId  	20151203030226

Steps to reproduce:
1. Install Firefox 44
2. Create several Panorama tab groups: two with specified names, and two with no name.
3. Update to Firefox 45

Expected results:
The "Tab Groups are no more. Sorry." page is opened and the tab groups are displayed with correct names.


Actual results:
The "Tab Groups are no more. Sorry." page is opened, but the groups with no name are displayed as Window 1 and Window 2.

	:Gijs (he/him) Assignee
	Comment 1 • 9 years ago

(In reply to Brindusa Tot from comment #0)
> Expected results:
> the tab groups are displayed with correct names.

> Actual results:
> the groups with no name are displayed as Window 1 and Window 2.

That *is* the correct name, if they had no name to begin with. What name were you expecting?

	Brindusa Tot, Desktop QA Reporter
	Comment 2 • 9 years ago

Sorry for not being clear.
As specified in bug 1221050 comment 16, the tab groups that don't have name should be named "Tab Group 1", "Tab Group 2" and so on.
Please see in the screenshot attached that in the bookmarks, the tab groups without names are named as "Group 1" and in the "Tab Groups are no more. Sorry." page are named Windows 1(attachment: https://bugzilla.mozilla.org/attachment.cgi?id=8696671)

	:Gijs (he/him) Assignee
	Comment 3 • 9 years ago

So the bookmark labels are correct and not the subject of this bug, but the restoration page is reusing about:sessionrestore's labels here, and I don't think it's worth extra engineering work in order to call these extra things "Group" rather than "Window" if they do get restored as windows. Getting confirmation from Philipp.

	Simona Badau, Desktop QA
	Comment 4 • 9 years ago

The order of the groups from the "Tab Groups are no more. Sorry." page is also confusing. For instance, I created 11 groups and I only named 6 of them. When they appear in the restoration page they are all mixed up (in the Bookmarks folder the first ones are named and are followed by the un-named ones).

	:Gijs (he/him) Assignee
	Comment 5 • 9 years ago

Bug 1231114 - fix labels of unnamed groups as well as group ordering on the tab migration page, r?mconley

	Mike Conley (:mconley) (:⚙️)
	Comment 6 • 9 years ago

Comment on attachment 8698171 [details]
MozReview Request: Bug 1231114 - fix labels of unnamed groups as well as group ordering on the tab migration page, r?mconley

https://reviewboard.mozilla.org/r/27849/#review25081

This looks okay to me (modulo nits). Thanks Gijs!

::: browser/modules/TabGroupsMigrator.jsm:121
(Diff revision 1)
> +              groupData.tabGroupsMigrationTitle = 

Trailing ws

::: browser/modules/test/xpcshell/test_TabGroupsMigrator.js:173
(Diff revision 1)
> -    Assert.equal(group2.tabGroupsMigrationTitle, "", "We don't assign titles to untitled groups");
> +    Assert.equal(group2.tabGroupsMigrationTitle, "Group 1", "We assign a numeric title to untitled groups");

So is the idea that even though the internal groupID of this group was "2", we assign it the title of "Group 1" because it was the first unnamed group we saw? If so, let's call this out explicitly in a comment above this assertion, since it's a bit strange to make sure that group2 has title "Group 1".

	:Gijs (he/him) Assignee
	Comment 7 • 9 years ago

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #6)
> ::: browser/modules/test/xpcshell/test_TabGroupsMigrator.js:173
> (Diff revision 1)
> > -    Assert.equal(group2.tabGroupsMigrationTitle, "", "We don't assign titles to untitled groups");
> > +    Assert.equal(group2.tabGroupsMigrationTitle, "Group 1", "We assign a numeric title to untitled groups");
> 
> So is the idea that even though the internal groupID of this group was "2",
> we assign it the title of "Group 1" because it was the first unnamed group
> we saw? If so, let's call this out explicitly in a comment above this
> assertion, since it's a bit strange to make sure that group2 has title
> "Group 1".

Good catch. Will do.

	Pulsebot
	Comment 8 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/f8840e2aec5a

	Carsten Book [:Tomcat]
	Comment 9 • 9 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f8840e2aec5a

	:Gijs (he/him) Assignee
	Comment 10 • 9 years ago

Comment on attachment 8698171 [details]
MozReview Request: Bug 1231114 - fix labels of unnamed groups as well as group ordering on the tab migration page, r?mconley

> Approval Request Comment
> [Feature/regressing bug #]: tab groups migration code
> [User impact if declined]: sorting and naming issues in the tab groups
> migration page
> [Describe test coverage new/current, TreeHerder]: has tests
> [Risks and why]: pretty low risk because of the tests, and also, aurora only
> just branched
> [String/UUID change made/needed]: no - I'm reusing existing strings so this
> is safe for aurora.

	Sylvestre Ledru [:Sylvestre]
	Comment 11 • 9 years ago

Comment on attachment 8698171 [details]
MozReview Request: Bug 1231114 - fix labels of unnamed groups as well as group ordering on the tab migration page, r?mconley

Has tests, feature removed

	Brindusa Tot, Desktop QA Reporter
	Comment 12 • 9 years ago

Verified as fixed on Firefox Nightly 46(upgrade from Nightly 44 to Nightly 46) on following OSes:
Windows 7 x64: buildID: 20151220030223, Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Ubuntu 14.04 x64: buildID: 20151221004011, Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Mac 10.10: buildID 20151221030239, Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:46.0) Gecko/20100101 Firefox/46.0

	Brindusa Tot, Desktop QA Reporter
	Comment 13 • 9 years ago

(In reply to Brindusa Tot from comment #12)
> Verified as fixed on Firefox Nightly 46(upgrade from Nightly 44 to Nightly
> 46) on following OSes:
> Windows 7 x64: buildID: 20151220030223, Mozilla/5.0 (Windows NT 6.1; WOW64;
> rv:46.0) Gecko/20100101 Firefox/46.0


By mistake added the Aurora Agent
> Ubuntu 14.04 x64: buildID: 20151221004011, Mozilla/5.0 (X11; Linux x86_64;
> rv:45.0) Gecko/20100101 Firefox/45.0
Actually verified on Nightly 46: 
BuildID: 20151220030223, Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0


> Mac 10.10: buildID 20151221030239, Mozilla/5.0 (Macintosh; Intel Mac OS X
> 10.10; rv:46.0) Gecko/20100101 Firefox/46.0

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 9 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/cb00a862858b

	Brindusa Tot, Desktop QA Reporter
	Comment 15 • 9 years ago

Verified as fixed on Aurora builds, on following Oses:
Windows 7 x64: 45.0a2 Build ID 20151229004007 - Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Ubuntu 14.04 x32: 45.0a2 Build ID20151228004010 - Mozilla/5.0 (X11; Linux i686; rv:45.0) Gecko/20100101 Firefox/45.0
Mac 10.10: 45.0a2 Build ID  20151229004007 - User Agent  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0) Gecko/20100101 Firefox/45.0