791311 - Dont disable the menubar for existing users

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Kent James (:rkent)]

Since bug 650170, existing users will suddenly find that there menubar has disappeared, which will lead to a lot of dissatisfaction.

I think that we should detect somehow that we are dealing with an existing user, and leave the menubar enabled for them. For new profiles we could still leave it disabled.

Comment 1

[Kent James (:rkent)]

I really don't think we should ship Thunderbird 17 without doing this.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

The solution to this is to simply remove the code I put in the migrateUI function:

http://mxr.mozilla.org/comm-central/source/mail/base/modules/mailMigrator.js#230

Comment 3

[Richard Marti (:Paenglab)]

What about the AppButton? Show it also for existing users? But then the two menu systems are shown together.

Should the AppButton also be only shown for new users and when the toolbars are resetted to default? Then by resetting also the menubar could be hidden as default.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I think Kent's idea is that for new users / new profiles, the AppMenu should be displayed, and the menu should be collapsed, as is current.

For existing users however, the AppMenu should be in the customization palette, and the menu should be in the state that the user has set it at (defaulting to displayed).

Users who reset their toolbars should get the AppMenu button displayed, and the menu bar hidden.

Do I have that right, Kent?

Comment 5

[Jim Porter (:squib)]

In the long run, isn't this just going to cause increasing divergence between existing users and new users, making support more difficult as we change things?

Besides, it's not as though this is change is totally out of left field. I expect that the majority of Thunderbird users also use Firefox, where the menubar has been hidden for a long time. We should be endeavoring to match Firefox's UI where possible, since it reduces context switching when moving between the two applications.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

(In reply to Jim Porter (:squib) from comment #5)
> We should be endeavoring to match
> Firefox's UI where possible, since it reduces context switching when moving
> between the two applications.

Yes, this is a very good point.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

I don't think that it's completely out of left field, but that it needs a fair bit of warning/messaging, since it's a change that breaks the user's flow, and isn't immediately obvious how to revert it.

(Also, we're still not quite like Firefox's UI, since we didn't add the Thunderbird button, and when Firefox adds the AppMenu button, they'll likely have a panel of icons, not the menu-like-thing we have.)

Kent, when you use Firefox, is your menubar hidden or showing?

Thanks,
Blake.

Comment 8

[Jim Porter (:squib)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #7)
> I don't think that it's completely out of left field, but that it needs a
> fair bit of warning/messaging, since it's a change that breaks the user's
> flow, and isn't immediately obvious how to revert it.

This would be a good opportunity for the old Migration Assistant to make a reappearance (perhaps as a tab, and perhaps serving double-duty with the "What's New" page).

Comment 9

[Wayne Mery (:wsmwk)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #7)
> I don't think that it's completely out of left field, but that it needs a
> fair bit of warning/messaging, since it's a change that breaks the user's
> flow, and isn't immediately obvious how to revert it.

yes. and I would offer that unlike firefox, IME (stress experience, not opinion) most unsophisticated users (and I dare say even sophisticated users here at univ.) make heavy use of the menus (and buttons). And are less familiar with context menu equivalent.

IMO, most casual users will be the same.

+1 to comment 8

Comment 10

[Mike Conley (:mconley) (:⚙️)]

(In reply to Jim Porter (:squib) from comment #8)
> (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #7)
> > I don't think that it's completely out of left field, but that it needs a
> > fair bit of warning/messaging, since it's a change that breaks the user's
> > flow, and isn't immediately obvious how to revert it.
> 
> This would be a good opportunity for the old Migration Assistant to make a
> reappearance (perhaps as a tab, and perhaps serving double-duty with the
> "What's New" page).

Just a note that this feature is currently landed in comm-aurora (TB 17), and with strings frozen, Migration Assistant is likely not possible.

We could, however, back out the migrateUI changes (but leave the button available in the customization palette), and have the Migration Assistant appear in a later version - 24, for instance...

Comment 11

[Kent James (:rkent)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #7)

> 
> Kent, when you use Firefox, is your menubar hidden or showing?
>

(FF15.0.1) my menubar is showing.

Comment 12

[Kent James (:rkent)]

I'm coming a little late to this party, so I am reacting more as a normal user.

What I would do is simply enable the menubar for existing users, and leave the app button as well. Yes that is a somewhat inconsistent state, but when they try the app button they will see that they don't need to menubar, and can disable it at their leisure.

To me this is much more "user friendly" than breaking their workflow on update, and forcing them to learn how to fix it when they would really just get their work done.

If you don't leave the app button showing, then you really do have a hidden important new feature, and it will make it even harder to migrate the users to the new interface.

There is no perfect answer, but I don't believe that making a large numbers of users angry is the correct approach.

Comment 13

[Blake Winton (:bwinton) (:☕️)]

(In reply to Kent James (:rkent) from comment #12)
> What I would do is simply enable the menubar for existing users, and leave
> the app button as well. Yes that is a somewhat inconsistent state, but when
> they try the app button they will see that they don't need to menubar, and
> can disable it at their leisure.

ui-r=me.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

My first run at this. Pushing to try so y'all can maybe try it out with some old profiles.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Try build coming in here: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=3e45d4cb6ce6

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 661334 [details] [diff] [review]
Patch v1

This seems to do the job.

Comment 17

[Joe Sabash [:JoeS1]]

The problem is:
How does leaving the app button there inform the user that the uncheck menubar/Alt to restore option even exists.
I think we need some kind of popup notification to inform about feature changes.

Comment 18

[Joe Sabash [:JoeS1]]

Hah..we have win32 builds now on try better linkhttp://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/[e-mail]-3e45d4cb6ce6/

Comment 19

[Joe Sabash [:JoeS1]]

(In reply to Joe Sabash from comment #18)
> Hah..we have win32 builds now on try better
> link http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> [e-mail]-3e45d4cb6ce6/

crap, I hate these new keyboards (space key sucks)
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> [e-mail]-3e45d4cb6ce6/

Comment 20

[Joe Sabash [:JoeS1]]

Third time is the charm
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/[e-mail]-3e45d4cb6ce6/

Comment 21

[Magnus Melin [:mkmelin]]

I don't think this is the correct fix, as new users will have a hard time anyway.

The problem is that we're not using what firefox uses, but what firefox will possibly use in the future - so, user's are not used to it from there and also, we have so many more buttons that it will *always* be hard to find (bug 790713). 

I'd really suggest just doing what firefox is doing right now (the firefox button in the window title) - that should be possible without string changes.

Comment 22

[Magnus Melin [:mkmelin]]

Especially for ubuntu (unity), the menu certainly should exist for maximized windows.

Comment 23

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 661334 [details] [diff] [review]
Patch v1

>+++ b/mail/base/modules/mailMigrator.js
>@@ -241,25 +241,16 @@ var MailMigrator = {
>             // Put the AppMenu button at the end.

I wonder if we should put the button at the end or at the start?

>+++ b/mail/test/mozmill/migration-to-rdf-ui-5/test-migrate-to-rdf-ui-5.js
>@@ -26,17 +26,9 @@ function test_appmenu_button_added() {
>-  // Skip the next test for OSX, since it never exposed the menubar.
>-  if (!mc.mozmillModule.isMac) {
>-    // Now also make sure that the menubar is collapsed.
>-    let menuBar = mc.e("mail-toolbar-menubar2");
>-    assert_equals(menuBar.getAttribute("autohide"), "true",
>-                  "The main menu should have autohide set to true.");
>-  }

Should we assert that if it was hidden, it still is, and if it was shown, it still is, instead?

I agree that Magnus's suggestion is a good one, but this patch still makes things better for existing users if no-one steps up to add the Thunderbird button in between now and when we release TB17.  So based on that, r=me.

Thanks,
Blake.

Comment 24

[Mike Conley (:mconley) (:⚙️)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #23)
> Comment on attachment 661334 [details] [diff] [review]
> Patch v1
> 
> >+++ b/mail/base/modules/mailMigrator.js
> >@@ -241,25 +241,16 @@ var MailMigrator = {
> >             // Put the AppMenu button at the end.
> 
> I wonder if we should put the button at the end or at the start?
> 

If Firefox is going to follow shorlander's designs, Firefox will be putting the menu on the right. If we operate on the assumption that most TB users are also FF users, I think any gains we make moving the button to the left is outweighed by the drastic change from FF to TB, where we have the same button on opposite sides for each app.

As usual, I'm open for argument. :)

> >+++ b/mail/test/mozmill/migration-to-rdf-ui-5/test-migrate-to-rdf-ui-5.js
> >@@ -26,17 +26,9 @@ function test_appmenu_button_added() {
> >-  // Skip the next test for OSX, since it never exposed the menubar.
> >-  if (!mc.mozmillModule.isMac) {
> >-    // Now also make sure that the menubar is collapsed.
> >-    let menuBar = mc.e("mail-toolbar-menubar2");
> >-    assert_equals(menuBar.getAttribute("autohide"), "true",
> >-                  "The main menu should have autohide set to true.");
> >-  }
> 
> Should we assert that if it was hidden, it still is, and if it was shown, it
> still is, instead?
> 

Since we're removing anything that has to do with showing / hiding the menubar, I think we'd just end up testing the XUL persistence stuff, which I believe is adequately tested in toolkit.

> I agree that Magnus's suggestion is a good one, but this patch still makes
> things better for existing users if no-one steps up to add the Thunderbird
> button in between now and when we release TB17.  So based on that, r=me.

Cool, thanks for the review!

Comment 25

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 661334 [details] [diff] [review]
Patch v1

We want this for TB 17 to avoid menubar-disappearing confusion.

Comment 26

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 661334 [details] [diff] [review]
Patch v1

Hold up - this patch doesn't hide the menubar for *new* users. Is that something we do want?

Withdrawing a? on Aurora until I hear more.

Comment 27

[David Lechner (:dlech)]

I have a suggestion. In the PDF software that I use, they recently switched to a ribbon style tool bar. The first time that the software ran after the update that added the ribbon interface, a dialog popped up. It showed a picture of the old menus and the new ribbon and let the user select if they wanted to keep the old or try the new. It also told you how to change it later if you changed your mind. Couldn't you do the same here?

Comment 28

[David Lechner (:dlech)]

Attachment: Screenshot of program with similar menu interface changes

This is a screen shot of the dialog that lets the user switch back and forth post-install. Iirc, the dialog that shows on the first run actually has more detailed information.

Comment 29

[Blake Winton (:bwinton) (:☕️)]

While I would have no problem with something like this, I don't believe the paid Thunderbird team has the resources to work on it.  However, I would be more than happy to review a patch that added it, if you attached one (or convinced a volunteer community member to attach one).

Thanks,
Blake.

Comment 30

[Jim Porter (:squib)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #29)
> While I would have no problem with something like this...

Aren't we past string freeze at this point?

Comment 31

[Mike Conley (:mconley) (:⚙️)]

(In reply to Jim Porter (:squib) from comment #30)
> (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #29)
> > While I would have no problem with something like this...
> 
> Aren't we past string freeze at this point?

If this is landing in TB 17, then yes, we're past string freeze, and a dialog that introduces new strings is not an option.

Comment 32

[Kent James (:rkent)]

(In reply to Mike Conley (:mconley) from comment #31)
> (In reply to Jim Porter (:squib) from comment #30)
> > (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #29)
> > > While I would have no problem with something like this...
> > 
> > Aren't we past string freeze at this point?
> 
> If this is landing in TB 17, then yes, we're past string freeze, and a
> dialog that introduces new strings is not an option.

That is why in tb-planning I have been arguing that we need a feature freeze period prior to ESR, with 17.1 becoming the last/ESR release with possible strings changes.

Comment 33

[Mike Conley (:mconley) (:⚙️)]

A feature freeze would be lovely. Not landing stuff on Aurora / Beta, doubly so.

As for this patch, here are the deficiencies:

1) This patch essentially adds the button, but does nothing else.
2) We want the patch to hide the menubar iff the user is a new user (or we're using a new profile)

I can't think of a good way of doing #2, except by simply checking to see if the user has 1 or more accounts set up. If the user has one or more accounts set up, we keep the menubar the way it's always been.

If the user has no accounts set up, we force autohide.

Does that sound good to everybody?

Comment 34

[Mike Conley (:mconley) (:⚙️)]

Unless I hear anything about this, the above comment is how I'm going to roll. I'll have a patch up tomorrow.

Comment 35

[Mike Conley (:mconley) (:⚙️)]

Another requirement - I'm going to add a pref that allows enterprises to specify that they don't want the menubar hidden when new accounts are created.

Comment 36

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2 - with tests!

This patch updates the last so that the autohide attribute of the main menu is not modified if:

a) The user has one or more accounts set up

or

b) The mail.mail_menu.collapse_by_default pref is set to false.

I've also added Mozmill tests for each case.

Pushing to try - results coming in here: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=d0d0f4f32250

Comment 37

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v3

Patch v2 went through try OK (there were unrelated test failures).

Just fixing a little documentation in this version.

Comment 38

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 662651 [details] [diff] [review]
Patch v3

>+++ b/mail/base/modules/mailMigrator.js
>@@ -223,17 +224,19 @@ var MailMigrator = {
>       // In UI version 5, we add the AppMenu button to the mail toolbar and
>-      // collapse the main menu by default.
>+      // collapse the main menu by default if the user has no accounts
>+      // set up (and the override pref "mail.main_menu.collapse_by_default"
>+      // is set to true).

I think we want to add a note about how this is a hack to guess whether this is a new install or not.

Other than that I'm pretty happy with this patch (as tested on Windows 7).

r=me.

Thanks,
Blake.

Comment 39

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 662651 [details] [diff] [review]
Patch v3

>+++ b/mail/base/modules/mailMigrator.js
>@@ -223,17 +224,19 @@ var MailMigrator = {
>       // In UI version 5, we add the AppMenu button to the mail toolbar and
>-      // collapse the main menu by default.
>+      // collapse the main menu by default if the user has no accounts
>+      // set up (and the override pref "mail.main_menu.collapse_by_default"
>+      // is set to true).

I think we want to add a note about how this is a hack to guess whether this is a new install or not.

Other than that I'm pretty happy with this patch (as tested on Windows 7).

r=me.

Thanks,
Blake.

Comment 40

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v4 (r+ from bwinton)

Cool, thanks for the review!

Requesting approval on comm-aurora since this change to the menubar was first introduced in TB 17.

Comment 41

[Mike Conley (:mconley) (:⚙️)]

comm-central: https://hg.mozilla.org/comm-central/rev/5241f7deed34

Marking dev-doc-needed because we need to document the mail.menu_menu.collapse_by_default pref that's now available to override the default behaviour.

Comment 42

[Mike Conley (:mconley) (:⚙️)]

comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/a65b4e961262