1889021 - Fix message list data properties and styles not correctly applied on startup

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect, P1)

Description

[Alessandro Castellani [:aleca]]

When launching Thunderbird, a stack trace for each of the message rows can be observed.

chrome://messenger/content/thread-card.mjs 81 set index
chrome://messenger/content/tree-view.mjs 1258 _addRowAtIndex
chrome://messenger/content/tree-view.mjs 1086 _ensureVisibleRowsAreDisplayed
chrome://messenger/content/tree-view.mjs 710 reset
chrome://messenger/content/tree-view.mjs 584 set view
chrome://messenger/content/about3Pane.js 4552 setTreeView
chrome://messenger/content/mailCommon.js 988 onCreatedView
resource:///modules/DBViewWrapper.sys.mjs 1544 _applyViewChanges
resource:///modules/DBViewWrapper.sys.mjs 1272 _enterFolder
resource:///modules/DBViewWrapper.sys.mjs 864 open
chrome://messenger/content/about3Pane.js 2511 _onSelect
chrome://messenger/content/about3Pane.js 1540 handleEvent
chrome://messenger/content/about3Pane.js 144

After a few seconds (inconsistent, sometimes it's 1 second, sometimes it's 2), another stack trace is triggered forcing all messages to be set again.

chrome://messenger/content/thread-card.mjs 81 set index
chrome://messenger/content/tree-view.mjs 1258 _addRowAtIndex
chrome://messenger/content/tree-view.mjs 1086 _ensureVisibleRowsAreDisplayed
chrome://messenger/content/tree-view.mjs 710 reset
chrome://messenger/content/mailCommon.js 1049 onMessagesLoaded
resource:///modules/DBViewWrapper.sys.mjs 1263 _enterFolder
resource:///modules/DBViewWrapper.sys.mjs 1250 _folderLoaded
resource:///modules/DBViewWrapper.sys.mjs 237 onFolderEvent

I noticed this because some data properties needed on the tree view weren't being applied right away.
The missing data attributes are due to the lack of window.threadPane.restoreThreadState(); in onCreatedView(), which is called in the first stack. The following stack uses onMessagesLoaded(), which applies all the correct styles.

Comment 1

[Alessandro Castellani [:aleca]]

Attachment: Bug 1889021 - Ensure tree table attributes are applied early, and improve the style of the Grouped by Sort headers. r=darktrojan,freaktechnik

Comment 2

[Alessandro Castellani [:aleca]]

Uploading an initial patch which takes care of missing style and some visual improvements.

Asking for some help in investigating this.
John, Hartmut, you've been doing some work recently on mailCommon and dbViewWrapper.

Could you investigate when the "folder loading twice at startup" issue started appearing?
This is a pretty noticeable performance regression.

Comment 3

[Alessandro Castellani [:aleca]]

Landing this initial fix and clean up, but leaving it open since the core problem still persists.

Comment 4

[[e-mail]]

See bug 1875655.

Comment 5

[Hartmut Welpmann [:welpy-cw]]

(In reply to Alessandro Castellani [:aleca] (PTO March 18-22) from comment #0)

The missing data attributes are due to the lack of window.threadPane.restoreThreadState(); in onCreatedView(), which is called in the first stack.

This had been changed in bug 1875577 to workaround the performance problem with Grouped by Sort in Expand-All state.

Regarding the "folder loading twice at startup" issue, I don't think there has been changed anything in the logic how this is handled in DBViewWrapper for quite some time.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1f0e21441163
Ensure tree table attributes are applied early, and improve the style of the Grouped by Sort headers. r=darktrojan

Comment 7

[Alessandro Castellani [:aleca]]

(In reply to Hartmut Welpmann [:welpy-cw] from comment #5)

This had been changed in bug 1875577 to workaround the performance problem with Grouped by Sort in Expand-All state.

Could you check if this change is regressing some of the performance aspects you tackled in that bug? https://hg.mozilla.org/comm-central/rev/1f0e21441163#l2.12

Comment 8

[Hartmut Welpmann [:welpy-cw]]

(In reply to Alessandro Castellani [:aleca] from comment #7)

(In reply to Hartmut Welpmann [:welpy-cw] from comment #5)

This had been changed in bug 1875577 to workaround the performance problem with Grouped by Sort in Expand-All state.

Could you check if this change is regressing some of the performance aspects you tackled in that bug? https://hg.mozilla.org/comm-central/rev/1f0e21441163#l2.12

Yes, with the latest artifact build, I tested a unified folder combining two local folders with more than 8000 messages in total, sorted by correspondents and grouped by sort. Selecting the folder in collapse-all state takes less than 2 sec, while in expand-all about 60 sec. After backing out this revision it is less than 2 sec in both cases. In a real folder with about 8000 messages there is no noticeable difference with or without this change.

BTW, something seems to be not quite right with the sort menu, sometimes the sort indicator is missing, and changing the sort (even by clicking the column header) doesn't work right away sometimes, but that seems to be independent of the changes in this revision.

Comment 9

[Alessandro Castellani [:aleca]]

(In reply to Hartmut Welpmann [:welpy-cw] from comment #8)

(In reply to Alessandro Castellani [:aleca] from comment #7)
Yes, with the latest artifact build, I tested a unified folder combining two local folders with more than 8000 messages in total, sorted by correspondents and grouped by sort. Selecting the folder in collapse-all state takes less than 2 sec, while in expand-all about 60 sec. After backing out this revision it is less than 2 sec in both cases. In a real folder with about 8000 messages there is no noticeable difference with or without this change.

Thanks, I will do a follow up and ping you during the review so we can find a good solution.

BTW, something seems to be not quite right with the sort menu, sometimes the sort indicator is missing, and changing the sort (even by clicking the column header) doesn't work right away sometimes, but that seems to be independent of the changes in this revision.

How old was your artifact?
Yesterday we land this fix for the sorting options, maybe it's related? https://hg.mozilla.org/comm-central/rev/4b3cd106f5c74e30da6ad8ad89fe632f962c9990

Comment 10

[Hartmut Welpmann [:welpy-cw]]

(In reply to Alessandro Castellani [:aleca] from comment #9)

How old was your artifact?
Yesterday we land this fix for the sorting options, maybe it's related? https://hg.mozilla.org/comm-central/rev/4b3cd106f5c74e30da6ad8ad89fe632f962c9990

Pulled and updated to https://hg.mozilla.org/comm-central/rev/02e053a8f57be4de6180effddabaf7f0dea2bddc, so that one was already included ...

Comment 11

[Alessandro Castellani [:aleca]]

Ah, damn.
Can you open a bug and ping Geoff (darktrojan) and John (TbSync) on it? Maybe we missed something.

Comment 12

[Hartmut Welpmann [:welpy-cw]]

(In reply to Alessandro Castellani [:aleca] from comment #11)

Ah, damn.
Can you open a bug and ping Geoff (darktrojan) and John (TbSync) on it? Maybe we missed something.

Will do later this evening.

By the way, just limiting this to real folders should fix the performance issue, while keeping the visual improvements.

Comment 13

[Alessandro Castellani [:aleca]]

Indeed, I forgot to add the condition for synthetic views.
I added it back and it solved the issue.
New patch coming up soon.

Comment 14

[Alessandro Castellani [:aleca]]

Attachment: Bug 1889021 - Ensure we don't try to restore expanded/collapsed state for synthetic views. r=#thunderbird-reviewers!,welpy-cw

Comment 15

[Alessandro Castellani [:aleca]]

Marking this to be closed after it lands since it tackles only styling and the message list loading twice is a completely different issue.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9be00f53256c
Ensure we don't try to restore expanded/collapsed state for synthetic views. r=vineet,welpy-cw

Comment 18

[Wayne Mery (:wsmwk)]

So this will help some cases of slowness when switching folders?

Comment 19

[Wayne Mery (:wsmwk)]

Should it also help when the message list is being updated during the receipt of new messages?

Comment 20

[Alessandro Castellani [:aleca]]

I don't think these things are related.
The issue here was mostly visual and not getting the correct properties to style the list if we were in grouped by sort or threaded.