Closed Bug 1484270 Opened 7 years ago Closed 6 years ago

Thunderbird 60: always starts up showing Account Central pane

Categories

(Thunderbird :: Mail Window Front End, defect)

Product:
Thunderbird
Component:
Mail Window Front End
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird_esr6869+ verified, thunderbird70 fixed, thunderbird71 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 71.0
Tracking Flags:
Tracking Status
thunderbird_esr68 69+ verified
thunderbird70 --- fixed
thunderbird71 --- fixed

People

(Reporter: walts48, Assigned: aceman)

References

(
URL
)

Details

(Keywords: regression)

User Story

https://support.mozilla.org/en-US/questions/1233908
https://support.mozilla.org/en-US/questions/1244947
https://support.mozilla.org/en-US/questions/1246444

Attachments

(1 file, 5 obsolete files)

1484270.patch v1.1
1.25 KB, patch
aceman
: review+
jorgk-bmo
: approval-comm-beta+
jorgk-bmo
: approval-comm-esr68+
Details | Diff | Splinter Review
+++ 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.
duplicate of bug 1393823?
Summary: Thunderbird 60: always starts up with the startup pane → Thunderbird 60: always starts up showing Account Central pane
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).
(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.
Walt, Do you see the same behavior with 56 beta? (i.e. bug 1393823)
URL: https://support.mozilla.org/en-US/que...
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(wls220spring)
See Also: → 1393823, 1481642
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
Flags: needinfo?(wls220spring)

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)

User Story: (updated)
Flags: needinfo?(wls220spring)

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.

Flags: needinfo?(wls220spring)

(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.

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.

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.

Attached patch await.patch (obsolete) — — Splinter Review

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.

Assignee: nobody → alta88

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.

Attachment #9036208 - Flags: feedback?(richard.marti)

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.

Attachment #9036208 - Flags: feedback?(richard.marti)

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.

Attachment #9036208 - Flags: feedback-
Attached patch await.patch (obsolete) — — Splinter Review

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

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

Attachment #9036208 - Attachment is obsolete: true
Attachment #9036220 - Attachment is obsolete: true
Attached patch await.patch (obsolete) — — Splinter Review

move verifyAccounts() to ensure gSummaryFrameManager is created.

Attachment #9036361 - Attachment is obsolete: true

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

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 :-(

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.

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

Keywords: regression
See Also: 1393823 →
Assignee: alta88 → nobody

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

Attachment #9036366 - Attachment is obsolete: true

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).

Yep, still present in 68.

Attached patch 1484270.patch (obsolete) — — Splinter Review

What about a quick hack?

Attachment #9091555 - Flags: feedback?(jorgk)
Attachment #9091555 - Flags: feedback?(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?
Attachment #9091555 - Flags: feedback?(alta88) → feedback+
Comment on attachment 9091555 [details] [diff] [review] 1484270.patch Not an expert here, but looks OK. So wha't next? r?who?
Attachment #9091555 - Flags: feedback?(jorgk)

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 ?

Flags: needinfo?(wls220spring)

(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.

Flags: needinfo?(wls220spring)

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.

(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 on attachment 9091555 [details] [diff] [review] 1484270.patch Review of attachment 9091555 [details] [diff] [review]: ----------------------------------------------------------------- Thank you, then we can continue here.
Attachment #9091555 - Flags: review?(mkmelin+mozilla)
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, " ?
Attachment #9091555 - Flags: review?(mkmelin+mozilla) → review+

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

Keywords: checkin-needed
Target Milestone: --- → Thunderbird 71.0

Thanks, fixed the comment.

Assignee: nobody → acelists
Attachment #9091555 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #9092704 - Flags: review+

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

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Comment on attachment 9092704 [details] [diff] [review] 1484270.patch v1.1 I guess you want backports here, right?
Attachment #9092704 - Flags: approval-comm-esr68?
Attachment #9092704 - Flags: approval-comm-beta+
Attachment #9092704 - Flags: approval-comm-esr68? → approval-comm-esr68+

v.fixed 68.1.1 per walt

status-thunderbird_esr68: fixed → verified

Confirm, that this is fixed in 68.1.1

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: