Closed Bug 1590036 Opened 5 years ago Closed 4 years ago

port bug 1518632 to Thunderbird - Show users who get a fresh profile instead of reusing the previous default a first run UI

Categories

(Thunderbird :: Preferences, task, P2)

Tracking

(thunderbird_esr68+ fixed, thunderbird74+ fixed)

RESOLVED FIXED
Thunderbird 75.0
Tracking Status
thunderbird_esr68 + fixed
thunderbird74 + fixed

People

(Reporter: mkmelin, Assigned: khushil324)

References

Details

Attachments

(2 files, 5 obsolete files)

Unfortunately we didn't notice that bug 1518632 did a change so that if you do get a new profile due to profile per install, you when you start Firefox you get an alert and a window explaining what happened, linking to a kb article and other such things. Try Firefox to see the details.

We should definitely implement this for Thunderbird too.

Try Firefox to see the details.

You get: "This installation of Firefox has a new profile." ... and more.

Note the amount of code that was landed for bug 1518632: https://hg.mozilla.org/mozilla-central/rev/a5ab8fa035ce

Assignee: nobody → khushil324
Attachment #9117952 - Flags: review?(mkmelin+mozilla)
Status: NEW → ASSIGNED
Comment on attachment 9117952 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile-0.patch

Review of attachment 9117952 [details] [diff] [review]:
-----------------------------------------------------------------

You'll have to look at the changelogs for relevant firefox files and port some of the changes

::: mail/base/content/aboutNewInstall.xhtml
@@ +10,5 @@
> +  <!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd">
> +  %brandDTD;
> +  <!ENTITY % aboutNewInstallDTD SYSTEM "chrome://messenger/locale/aboutNewInstall.dtd">
> +  %aboutNewInstallDTD;
> +]>

Things changed since the original landing for Firefox. So this is now newInstallPage.html
I think it makes sense to go directly to that instead. It would get Fluent for localization at the same time.

@@ +32,5 @@
> +          <h3>&newInstall.optionsTitle;</h3>
> +          <p>&newInstall.optionsDoNothing;</p>
> +          <p>
> +            <span>&newInstall.resources;</span><br/>
> +            <a href="https://support.mozilla.org/kb/profile-manager-create-and-remove-firefox-profiles" target="_blank" rel="noopener">&newInstall.supportLink;</a>

Please use https://support.mozilla.org/kb/profile-manager-create-and-remove-thunderbird-profiles
We'll have to create such a page.

::: mail/base/content/newInstall.xul
@@ +6,5 @@
> +<!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd">
> +%brandDTD;
> +<!ENTITY % newInstallDTD SYSTEM "chrome://messenger/locale/newInstall.dtd">
> +%newInstallDTD;
> +]>

xul files are no longer working

::: mail/components/MailGlue.jsm
@@ +230,5 @@
> +          });
> +        }
> +      } catch (e) {
> +        dump(e);
> +      }

in general, it's preferable to try to keep in sync with the firefox code wrt naming and such. Like having a howNewInstallModal  function. Easier to keep up with future ports then, even if the content of the functions would not be completely identical

::: mail/locales/en-US/chrome/messenger/aboutNewInstall.dtd
@@ +4,5 @@
> +
> +<!ENTITY newInstall.title "Important News">
> +<!ENTITY newInstall.heading "Changes to your &brandShortName; profile">
> +<!ENTITY newInstall.changedTitle "What changed?">
> +<!ENTITY newInstall.changedDescProfiles "This installation of &brandShortName; has a new profile. A profile is the set of files where Thunderbird saves information such as bookmarks, passwords, and user preferences.">

where it says Thunderbird, that should be (in Fluent), { -brand-product-name }. In all of the file. Firefox has that wrong.

... such as email data, passwords, and user preferences.

@@ +5,5 @@
> +<!ENTITY newInstall.title "Important News">
> +<!ENTITY newInstall.heading "Changes to your &brandShortName; profile">
> +<!ENTITY newInstall.changedTitle "What changed?">
> +<!ENTITY newInstall.changedDescProfiles "This installation of &brandShortName; has a new profile. A profile is the set of files where Thunderbird saves information such as bookmarks, passwords, and user preferences.">
> +<!ENTITY newInstall.changedDescDedicated "In order to make it easier and safer to switch between installations of Thunderbird (including Thunderbird, Thunderbird ESR, Thunderbird Beta, and Thunderbird Nightly), this installation now has a dedicated profile. It does not automatically share your saved information with other Thunderbird installations.">

including { -brand-product-name }, { -brand-product-name } Beta, and { -brand-product-name } Daily
Attachment #9117952 - Flags: review?(mkmelin+mozilla) → review-

Should we adjust the text here a little? "A profile is the set of files where Thunderbird saves information such as bookmarks, passwords, and user preferences" - hmm, bookmarks in Thunderbird? Never seen one. We should mention mail accounts, mail data and address books and consequences of losing everything when a new profile is created. Can we point out "Help > Troubleshooting Information, about:profiles" and the ability to get back to the original profile?

Yeah, it needs some adjustment like i wrote, "such as email data, passwords, and user preferences". Sure, let's include address books too.

Maybe we can link to about:profiles directly?

Well, and setting up accounts again is a hassle, too. Some people don't care, they can set up their Gmail account in 30 seconds and re-download their mail. For others who have plenty of accounts, including pop but also also feeds, news or chat, it's much harder.

(In reply to Magnus Melin [:mkmelin] from comment #3)

Things changed since the original landing for Firefox. So this is now
newInstallPage.html
I think it makes sense to go directly to that instead. It would get Fluent
for localization at the same time.

That page has a division related to Firefox Account which is not relevant to us. Checkout about:newinstall. I find many differences so I chose to have this file here.

(In reply to Magnus Melin [:mkmelin] from comment #5)

Maybe we can link to about:profiles directly?

Nice idea, I will add a link in resources.

(In reply to Khushil Mistry [:khushil324] from comment #7)

That page has a division related to Firefox Account which is not relevant to us. Checkout about:newinstall. I find many differences so I chose to have this file here.

There is, but since it's still the corresponding page, please use the same name, and only strip out the things not relevant to us.

Attachment #9117952 - Attachment is obsolete: true
Attachment #9117985 - Flags: review?(mkmelin+mozilla)
Attachment #9117985 - Attachment is patch: true
Comment on attachment 9117985 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile-1.patch

Review of attachment 9117985 [details] [diff] [review]:
-----------------------------------------------------------------

Please update your tree. The patch doesn't apply.

::: mail/components/MailGlue.jsm
@@ +240,5 @@
>        AppUpdateUI.init();
>      }
>    },
>  
> +  _howNewInstallModal() {

show, no how

@@ +245,5 @@
> +    let pService = Cc["@mozilla.org/toolkit/profile-service;1"].getService(
> +      Ci.nsIToolkitProfileService
> +    );
> +
> +    if (pService.createdAlternateProfile) {

I think you should mirror https://searchfox.org/mozilla-central/rev/a46d0fc0e7bf06b64a8d27e2cdbeeee94ecc7ad1/browser/components/BrowserGlue.jsm#1994 more here, that is, the if in the caller, and naming.

 I'm not sure which windows should be shown first. newInstall.xhtml or the main window.

::: mail/locales/en-US/chrome/messenger/newInstallPage.dtd
@@ +11,5 @@
> +<!ENTITY newInstall.optionsTitle "What are my options?">
> +<!ENTITY newInstall.optionsDoNothing "If you do nothing, your profile data in &brandShortName; will be different from profile data in other installations of &brandProductName;.">
> +<!ENTITY newInstall.resources "Resources:">
> +<!ENTITY newInstall.supportLink "Using the Profile Manager - Support Article">
> +<!ENTITY newInstall.profileLink "About Profiles">

Maybe "See and Manage Your Profiles"
And make it aboutPofilesLink

Although Firefox didn't migrate this page to Fluent yet, maybe you can still do that now directly.
Attachment #9117985 - Flags: review?(mkmelin+mozilla)
Attachment #9118071 - Attachment is obsolete: true
Attachment #9118071 - Flags: review+
Comment on attachment 9118140 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile-2.patch

Review of attachment 9118140 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/newInstallPage.xhtml
@@ +1,1 @@
> +<?xml version="1.0" encoding="UTF-8"?>

Firefox moved this to .html

@@ +18,5 @@
> +  <body>
> +    <div id="main">
> +      <div id="content">
> +        <div id="info">
> +          <h1 data-l10n-id="heading"/>

Even for xhtml, self closing tags are no good... unless it's the few special cases like <img/>

::: mail/components/MailGlue.jsm
@@ +243,5 @@
>  
> +  _showNewInstallModal() {
> +    let pService = Cc["@mozilla.org/toolkit/profile-service;1"].getService(
> +      Ci.nsIToolkitProfileService
> +    );

Please align with firefox, that is, at caller do 
if (pService.createdAlternateProfile) {
  _showNewInstallModal()
}

What about my other questions regarding this? (ordering)

@@ +263,5 @@
> +            clickHandler: "specialTabs.aboutClickHandler(event);",
> +          });
> +        }
> +      } catch (e) {
> +        dump(e);

what are we catching?

::: mail/locales/en-US/messenger/newInstallPage.ftl
@@ +11,5 @@
> +options-title = What are my options?
> +options-do-nothing = If you do nothing, your profile data in { -brand-short-name } will be different from profile data in other installations of { -brand-product-name }.
> +resources = Resources:
> +support-link = Using the Profile Manager - Support Article
> +about-pofiles-link = See and Manage Your Profiles

Please change order to be the same as in the html file (move about-pofiles-link  up one row)
Attachment #9118140 - Flags: review?(mkmelin+mozilla)

(In reply to Magnus Melin [:mkmelin] from comment #14)

What about my other questions regarding this? (ordering)

First, we want to pop up the alert dialog. If we click on continue then we will be redirected to the HTML page with more information and related links if we want to change the profiles. If we first show the HTML page then dialog will be of no use. We can remove the dialog as the same information is shown on the HTML page but Firefox is showing both the things so we want to be consistent with Firefox behavior.

Attachment #9118140 - Attachment is obsolete: true
Attachment #9119048 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9119048 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile-3.patch

Review of attachment 9119048 [details] [diff] [review]:
-----------------------------------------------------------------

I've looked at this in more detail now. It looks like it does what Firefox does, that doesn't fit in too well with how the initial startup flow goes for Thunderbird: the dialog pops up behind stuff (in Firefox too), but for Thunderbird it's also a problem that we have the new account setup popping up so there are multiple dialogs stacked an no sense to make of it. 
I'll a attach a modified patch that just adds the code to our startup flow and avoids popping up the account wizard at the "wrong" time.
Attachment #9119048 - Flags: review?(mkmelin+mozilla)

Slightly modified

Attachment #9120732 - Flags: review?(khushil324)
Attachment #9119048 - Attachment is obsolete: true

I wish I had kept better track of this. Given it is a frequent - and so important - support issue we should a) make it a priority for finishing and landing in beta and b) get this into version 68, with or without translation. We still have several million users who haven't updated to version 68, some of whom will hit this issue and potentially benefit.

I haven't tested the code or how Firefox behaves, but looking at our patch I find this language to be confusing. Particularly, it seems to refer to profiles as "installations". While that may be the technical term used by the folk in bug 1518632, this feels like the wrong terminology for release users. I think it requires a UX touch.

We're also going to need the support articles https://support.mozilla.org/en-US/kb/dedicated-profiles-thunderbird-installation and https://support.mozilla.org/en-US/kb/dedicated-profiles-firefox-installation

Before I forget, to test this, go into profiles.ini and delete the entry for the profile that the install you just run created.

Comment on attachment 9120732 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile.patch

Review of attachment 9120732 [details] [diff] [review]:
-----------------------------------------------------------------

It works for me. I will make a patch for ESR 68. Do we want to change anything here? Any thoughts on Wayne's comment?
Attachment #9120732 - Flags: review?(khushil324) → review+

(In reply to Wayne Mery (:wsmwk) from comment #19)

I haven't tested the code or how Firefox behaves, but looking at our patch I find this language to be confusing. Particularly, it seems to refer to profiles as "installations".

It's not directly doing that, but I agree we should add some more information and clarification here.

Wayne, can you take on setting up the https://support.mozilla.org/en-US/kb/dedicated-profiles-thunderbird-installation article adapted from the corresponding Firefox one?

To see what Firefox has, go to about:newinstall

Flags: needinfo?(vseerror)

We also need to have the profile article updated to use about:profiles. instead of the old profile manager. It would be good to have a good user interface for accessing that when re-writing the article.

The first link under "Resources" opens about:profiles

Maybe I should get this landed and we can iterate if needed.

(In reply to Magnus Melin [:mkmelin] from comment #27)

Maybe I should get this landed and we can iterate if needed.

Makes sense

(In reply to Magnus Melin [:mkmelin] from comment #24)

How about this: https://pasteboard.co/IQdBPNy.png

Good enough start.

Flags: needinfo?(vseerror)
Target Milestone: --- → Thunderbird 75.0

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/581b8fbdb8e2
Show users who get a fresh profile instead of reusing the previous default a first run UI (about:newinstall). r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

We might want to take this for beta, and ESR. For ESR a new patch is needed (change .xhtml to .xul, rebase, and also put the strings in a non-localizable place)

Attachment #9120732 - Flags: approval-comm-beta?
Comment on attachment 9120732 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile.patch

Let's get some experience with it and iterate
Attachment #9120732 - Flags: approval-comm-beta? → approval-comm-beta+
Attachment #9129840 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9129840 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile-ESR68.patch

Review of attachment 9129840 [details] [diff] [review]:
-----------------------------------------------------------------

Seems to work. r=mkmelin

::: mail/base/content/newInstall.xul
@@ +17,5 @@
> +  <hbox align="start" flex="1">
> +    <image id="alert" role="presentation"/>
> +    <vbox align="end" flex="1">
> +      <description class="main-text">
> +        This installation of Thunderbird has a new profile.

The idea was actually to just adjust the path for the localized files (move them to content), like https://searchfox.org/comm-central/rev/365b1519f0659eb59d785804c7372d525f8334d1/mail/locales/jar.mn#12-14
But since you already did this now, I guess we can just do it with the hardcoded strings.
Attachment #9129840 - Flags: review?(mkmelin+mozilla) → review+
Attachment #9129840 - Flags: approval-comm-esr68?
Comment on attachment 9129840 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile-ESR68.patch

Reminder we need to stand up https://support.mozilla.org/kb/profile-manager-create-and-remove-thunderbird-profiles
Flags: needinfo?(vseerror)
Flags: needinfo?(unicorn.consulting)
Attachment #9129840 - Flags: approval-comm-esr68? → approval-comm-esr68+

(In reply to Wayne Mery (:wsmwk) from comment #35)

Comment on attachment 9129840 [details] [diff] [review]
Bug-1590036_popup-on-fresh-profile-ESR68.patch

Reminder we need to stand up
https://support.mozilla.org/kb/profile-manager-create-and-remove-thunderbird-
profiles

I have stood up a shell document, but I have no idea what content is expected in it, or why it exists other than the note in this bug.

Link here https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-thunderbird-prof/history
I suggest someone that knows what this bug is about actually fills in some content because I am not even able to follow what the expected content is. I think this is a duplication process. We are creating a new article because software developers are copying verbatim without regard to existing documentation. But as I don't understand what is going on here, perhaps there really is something new and relevant to put in a dedicated article.

Firefox has https://support.mozilla.org/en-US/kb/profile-manager-create-remove-switch-firefox-profiles
Thunderbird has https://support.mozilla.org/en-US/kb/profiles-where-thunderbird-stores-user-data

I am not happy to be looking at documenting the old profile manager using command line switches such as Firefox do in their article. About:profiles is for creating and removing profiles. Even if we have a clunky access method it is still eminently more suited to GUI users than a command line switch.

Flags: needinfo?(unicorn.consulting)

Is it planned to release the article before the 78 release? SUMO localizers also need some time to get it ready.

Flags: needinfo?(vseerror)

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #39)

Is it planned to release the article before the 78 release? SUMO localizers also need some time to get it ready.

THanks for asking.

Yes. Anticipate in the next day or so. Do they normally also localize images?

(In reply to Wayne Mery (:wsmwk) from comment #40)

Yes. Anticipate in the next day or so. Do they normally also localize images?

At least for German they do.

Flags: needinfo?(vseerror)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: