1484270 - Thunderbird 60: always starts up showing Account Central pane

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[WaltS48 [:walts48]]

+++ This bug was initially created as a clone of Bug #1481642 +++

I did some testing and noticed the problem started occurring in Thunderbird 60.0b1(build1) 64-bit on Ubuntu 18.04.1 LTS and 32-bit on Windows 10.

In each version the Account Pane only appears briefly before the Inbox Thread Pane appears.

I also tested 45.8.0 and 52.9.1, where the problem did not present itself.

New separate profiles for each version, one email account, only extension is Lightning.

Disabling Lightning and restarting doesn't solve the problem.

Don't know if this helps.

buildid: 20180315080846
builduid: bba9867a909f4ffc99bbcb0033b00975
revision: ea8dbf162c4b

Steps to reproduce:

Start Thunderbird.
Disable Lightning and restart.
Create an email account and get messages.
Select the Inbox and Quit Thunderbird.
Start Thunderbird again.

What happens:

Thunderbird opens and the Account Pane briefly appears before the Thread Pane for the Inbox appears.

What should happen:

Thunderbird should open with the Inbox Thread Pane and messages appearing.

Comment 1

[Wayne Mery (:wsmwk)]

duplicate of bug 1393823?

Comment 2

[Jorg K (GMT+2)]

As I said in bug 1481642 comment #6 (quote): "Yes, the startup page (note: not correct) will show for a short while before the previous status is restored. I don't think we will action this ...". But if anyone wants to investigate this and submit a patch, we'll consider it.

What I think happens is that TB starts up and shows a Account Central as default, and after a short delay it displays the previously open tab (folder, message, options (in tab), add-on manager, calendar, or so).

Comment 3

[WaltS48 [:walts48]]

(In reply to Wayne Mery (:wsmwk) from comment #1)
> duplicate of bug 1393823?

Possibly, but in TB 60.0 the problem also exists in safe mode.

Comment 4

[Wayne Mery (:wsmwk)]

Walt, Do you see the same behavior with 56 beta?  (i.e. bug 1393823)

Comment 5

[WaltS48 [:walts48]]

Tested again and still don't see it in Safe Mode with 56.0b4.

https://bugzilla.mozilla.org/show_bug.cgi?id=1393823#c3

Does occur in Safe Mode with TB 60.0

Comment 9

[Wayne Mery (:wsmwk)]

Walt, can you test nightlies to find a one day regression range where this got worse?
(I realize the behavior isn't a regression, but the "severity" is)

Comment 10

[WaltS48 [:walts48]]

55.0a1 Build ID: 20170501030205 is good.
Built from https://hg.mozilla.org/comm-central/rev/4d0138629df2a1b053836b59d50c33dc7d48fbbc

55.0a1 Build ID: 20170502030225 is the first nightly to display Account Central with the Inbox selected on startup..
Built from https://hg.mozilla.org/comm-central/rev/a7d9e926d66e7d79b2292945d7897cb5c9efe553

Not sure why I didn't see it in 56.0b4 Comment 5.

Comment 11

[Wayne Mery (:wsmwk)]

Excellent. That gives us https://hg.mozilla.org/comm-central/pushloghtml?fromchange=4d0138629df2a1b053836b59d50c33dc7d48fbbc&tochange=a7d9e926d66e7d79b2292945d7897cb5c9efe553

So bug 1358906 introduces a performance regression?

Comment 12

[alta88]

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

Excellent. That gives us https://hg.mozilla.org/comm-central/pushloghtml?fromchange=4d0138629df2a1b053836b59d50c33dc7d48fbbc&tochange=a7d9e926d66e7d79b2292945d7897cb5c9efe553

So bug 1358906 introduces a performance regression?

No, in fact the opposite. Making it async means the window appears faster and paints the default view AC, while fetching the session file. Fixing the brief AC flash would mean waiting for the file read, but would be worth it as ui flash is bad. It could also possibly be fixed with css trickery.

Comment 13

[Jorg K (GMT+2)]

So the effect is "psychological":
Before: Sync wait for the session file, then display the last session state.
Now: Display A/C (although unwanted), when async result comes back, display the last session state.

Comment 14

[alta88]

Yep. All recent animations and jank reduction are tricks on the user. In fact, merely having a throbber/progressbar will trick the user into not feeling UI is slow, even though something pops up with the same delay without them.

I don't see this on linux with any of the standard 3 layouts, but do on win7.

Comment 15

[alta88]

Attachment: await.patch

This affects my extension MoreLayouts with widethread view on linux, so.. This patch seems to work for that view, but should be built on win. If you can give it a try jorg.

Comment 16

[Jorg K (GMT+2)]

Comment on attachment 9036208 [details] [diff] [review]
await.patch

Thanks for the patch, this looks good. Unfortunately my slow local debug build doesn't show the problem, when I start, I'm immediately in the last folder opened, no A/C in sight.

Richard, you do local opt builds, can you please check the patch, in case you usually see A/C when you start.

Comment 17

[Richard Marti (:Paenglab)]

Comment on attachment 9036208 [details] [diff] [review]
await.patch

I don't see a difference. I've never seen the account central before the thread pane. Tested also on different VM's all I see is a white or grey background, depending on the OS, before the thread pane appears. And this hasn't changed with the patch.

Comment 18

[Jorg K (GMT+2)]

Attachment: await.patch - ESR60 version

OK, I'll try it myself then with a TB 60 ESR build:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=edab1ab67aed170d3a5cde92f51988b47db2f41a

Comment 19

[Jorg K (GMT+2)]

Comment on attachment 9036208 [details] [diff] [review]
await.patch

I hate to hand out f-, but I tried it on the ESR and A/C still flashes up at startup. Just briefly, but just as much as "ordinary" ESR 60. I'll report again on a "cold start" after a reboot of the system in the morning.

Comment 20

[alta88]

Attachment: await.patch

if it doesn't work, it doesn't work ;)

can you try this one (added await in the main onload function).

Comment 21

[alta88]

Attachment: await.patch

move verifyAccounts() to ensure gSummaryFrameManager is created.

Comment 22

[alta88]

removing -purgecaches makes this more obvious on linux; the latest patch doesn't completely fix it either (starting up with vertical view).

Comment 23

[Jorg K (GMT+2)]

You still want me to try? Linting changed all JS files on trunk in mail/, so I have to rebase the patch every time for 60 ESR before shipping it off to try :-(

Comment 24

[wroot]

If you can provide me Windows installer with a patch and i would be able to upgrade to an official version later (if not, i can maybe do a backup of my profile and revert after testing), i can test this on Windows 7 x64. I can reproduce the issue currently.

Comment 25

[alta88]

No, it isn't fixed for me, and doesn't seem like an easy fix.

Comment 28

[alta88]

Parts of this patch are in Bug 1554237 to fix a different thing, it's hard to tell if this problem is solved..

Comment 29

[:aceman]

The problem still exists, it is even confirmed by bug 1568440, trying to solve the AC being redrawn twice (but that bug is probably for the case where the Account central is intended to stay displayed as an account is selected).

In my test in bug 1579575, the account central is still called twice on startup, once without any folder selected, then with the right selection.
We can catch the case where there is nothing selected, but that is a valid state when there are no accounts yet (new profile) and we need to distinguish that from the case where the folder tree just wasn't yet built (or session restored).

Comment 30

[wroot]

Yep, still present in 68.

Comment 31

[:aceman]

Attachment: 1484270.patch

What about a quick hack?

Comment 32

[alta88]

Comment on attachment 9091555 [details] [diff] [review]
1484270.patch

Not even a hack, nice. Ok on visual inspection, and I assume you tested restarts where both a message folder an account folder are selected/restored, and also tab and layout changes.

It would also be nice to have an 'are we snappy yet' initiative (other than handwaving). The async session restore went a way there, but there could be possibly more snappiness to startup by moving some things around in the startup path. For example, is the very first thing window onLoad should do is TagUtils.loadTagsIntoCSS() or is that better in completeStartup() after window paint?

Comment 33

[Jorg K (GMT+2)]

Comment on attachment 9091555 [details] [diff] [review]
1484270.patch

Not an expert here, but looks OK. So wha't next? r?who?

Comment 34

[:aceman]

Yes I tested the final result of what stays displayed. But I didn't see the original bug in the first place, probably on a debug build it is so slow, that the session restore is done before the paint of the main window is shown. So I didn't see the flashing of AC then thread pane.

Walt, do you have a test profile to check a build of TB 71 ?

Comment 35

[WaltS48 [:walts48]]

(In reply to :aceman from comment #34)

Yes I tested the final result of what stays displayed. But I didn't see the original bug in the first place, probably on a debug build it is so slow, that the session restore is done before the paint of the main window is shown. So I didn't see the flashing of AC then thread pane.

Walt, do you have a test profile to check a build of TB 71 ?

I still see a short flash of Account Central with an accounts Inbox, other folder, a feed or newsgroup selected.

I just updated my 71.0a1 to Build ID 20190911095508 Mozilla/5.0 (X11; Linux x86_64; rv:71.0) Gecko/20100101 Thunderbird/71.0a1 Ubuntu 18.04.3 LTS.

Comment 36

[:aceman]

We haven't landed this in trunk, so try the build with the patch from here: https://queue.taskcluster.net/v1/task/KFmv1gngTzCNy-6hfBTUyA/runs/0/artifacts/public/build/target.tar.bz2 . Thanks.

Comment 37

[WaltS48 [:walts48]]

(In reply to :aceman from comment #36)

We haven't landed this in trunk, so try the build with the patch from here: https://queue.taskcluster.net/v1/task/KFmv1gngTzCNy-6hfBTUyA/runs/0/artifacts/public/build/target.tar.bz2 . Thanks.

That one looks good with a new test profile, one email and newsgroup account created with several quits and restarts.

Comment 38

[:aceman]

Comment on attachment 9091555 [details] [diff] [review]
1484270.patch

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

Thank you, then we can continue here.

Comment 39

[Magnus Melin [:mkmelin]]

Comment on attachment 9091555 [details] [diff] [review]
1484270.patch

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

::: mail/base/content/folderDisplay.js
@@ +1808,5 @@
>     *  central page.
>     */
>    _showAccountCentral() {
> +    if (!this.displayedFolder && MailServices.accounts.accounts.length > 0) {
> +      // If there exist any accounts, but no folder was selected yet,

grammar derailed here

"If we have some account set up, but no folder is selected yet, " ?

Comment 40

[Jorg K (GMT+2)]

I'll land it later tonight and fix the comment.

Comment 41

[:aceman]

Attachment: 1484270.patch v1.1

Thanks, fixed the comment.

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f8986e7f6f2e
Do not load Account Central until we have a folder selected on startup. r=mkmelin

Comment 43

[Jorg K (GMT+2)]

Comment on attachment 9092704 [details] [diff] [review]
1484270.patch v1.1

I guess you want backports here, right?

Comment 44

[Jorg K (GMT+2)]

TB 70 beta 2:
https://hg.mozilla.org/releases/comm-beta/rev/4d812d0a85aca34312b73ffec795818186b3d484

Comment 45

[Jorg K (GMT+2)]

TB 68.1.1 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/73214b3f677012ca0e5a5f15f9ce614f13ae9905

Can someone please check that this works better now.

Comment 46

[Wayne Mery (:wsmwk)]

v.fixed 68.1.1 per walt

Comment 47

[wroot]

Confirm, that this is fixed in 68.1.1