1817682 - No way for extensions to add columns to the new tree list widgets

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, defect, P1)

Description

[Mark Banner (:standard8)]

I took a look at the new tree list widgets for Thunderbird in 111a1+ with the intention of seeing how they can be extended from a WebExtension perspective.

If I'm understanding these comments correctly, then it is not possible to dynamically add columns to these tree views which means the WebExtension case is currently on non-starter.

There are lots of add-ons that add columns to the various views (Thunderbird Conversations is also one).

Comment 1

[Mark Banner (:standard8)]

Note: ideally anything provided here would also help towards providing real WebExtension APIs for adding columns (bug 1615801). Doing both together would also avoid WebExtension authors having to do two migrations.

Comment 2

[Alessandro Castellani [:aleca]]

(In reply to Mark Banner (:standard8) (afk until 27 Feb) from comment #0)

If I'm understanding these comments correctly, then it is not possible to dynamically add columns to these tree views which means the WebExtension case is currently on non-starter.

These comments are a bit outdated (already) since the setColumns() is used whenever it's needed, even in between folders if the structure is completely different.

For now, web experiments can grab a copy of the default columns, add or change what they want, and trigger an update.

Once the new about3Pane is more stable and we handled all the features and regressions, we will create a plan to add extensions API to add columns.

Comment 3

[Mark Banner (:standard8)]

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

For now, web experiments can grab a copy of the default columns, add or change what they want, and trigger an update.

I've been trying this, and I've so far hit a few issues:

• The L10n set-up assumes strings are provided by Fluent. This won't work for WebExtensions since they will provide their own strings.
• I've been trying code like the snippet below, however it seems even though the column is in DEFAULT_COLUMNS, it is only present on the initially selected folder. Switching to other folders (and even back to the original) doesn't show it.

  if (!DEFAULT_COLUMNS.find((c) => c.id == "betweenCol")) {
    DEFAULT_COLUMNS.push({
      id: "betweenCol",
      ordinal: 23,
      sortKey: "byBetween",
      l10n: { header: columnTooltip, menuitem: columnName },
    });
  }
  treeTable.setColumns(DEFAULT_COLUMNS);

• I've been attempting to insert items into the column using the traditional addColumnHandler, example code here. However, this also doesn't seem to work.

Do you have any hints about these, or will they need code fixes?

Once the new about3Pane is more stable and we handled all the features and regressions, we will create a plan to add extensions API to add columns.

One thing I will note that if we're still using nsIMsgCustomColumnHandler/nsITreeView combo in their current form, then I expect the performance to be quite poor especially with the new tree view - depending on how the API is created. If it simply echos what is there, then iirc there would be four or five calls across the interface per cell, hence this will need some consideration.

Comment 4

[Alessandro Castellani [:aleca]]

Do you have any hints about these, or will they need code fixes?

Yes to both :D

After you modify the columns, you should call all these methods.
That will ensure that the updated columns array is persisted in the xulStore of the current folder.
The other methods will take care of updating the tree and invalidating the rows.

Then, if you want to propagate this new columns array and order to all other folders, you can call this method.
The destFolder should be the root folder of your account, or loop through all accounts and pass the root folder so every folder gets the new order.

We're still improving the code and expanding the concept of the DEFAULT_COLUMNS in order to offer more control in tweaking that array before anything is set.
Most likely we will not use nsIMsgCustomColumnHandler, but that still needs to be defined. We will have a more clear path in the next few weeks.

Comment 5

[Alessandro Castellani [:aleca]]

There's also ongoing work to make the DEFAULT_COLUMNS be more flexible and allow injecting stuff in there, or dynamically change them based on the folder flags: https://phabricator.services.mozilla.com/D167900

Comment 6

[Geoff Lankow (:darktrojan)]

(In reply to Mark Banner (:standard8) (afk until 27 Feb) from comment #3)

One thing I will note that if we're still using nsIMsgCustomColumnHandler/nsITreeView combo in their current form, then I expect the performance to be quite poor especially with the new tree view - depending on how the API is created. If it simply echos what is there, then iirc there would be four or five calls across the interface per cell, hence this will need some consideration.

I think we're going to stop doing that and pull the message header object across XPCOM instead, because it's one call per row instead of many. But we have more important things to deal with for now.

Comment 7

[Mark Banner (:standard8)]

Thanks for the feedback. I'll stop playing for now as this all seems to be in flux, but heading in the right direction :)

The only thing I would add is that it would be useful if we could know when WebExtensions can test via experiments (at least), and preferably for that time to be a little while ahead of the next release in case of issues.

Comment 8

[Alessandro Castellani [:aleca]]

We now have a handy function to get the default columns when loading a folder.

We will probably add an API entry point to add and remove the DEFAULT_COLUMNS array, but for now I think things can be tweaked via experiments.
Some discussions need to happen to properly define this, but we should have something to allow add-on devs to update their code around the end of April. It's a tentative timeline, so don't quote me on this :D

Comment 9

[Geoff Lankow (:darktrojan)]

I have a working prototype.

I think what an add-on will need to do is import the columns module and register a column handler. The module can do the rest.

A column handler would need to provide:

• text for a cell given an nsIMsgDBHdr
• a sorting value (string/integer) given an nsIMsgDBHdr
• some name field to use in the UI

This would give an add-on the ability to put text in a cell and sort by that column. Am I missing any of the old abilities that are wanted?

Comment 10

[Mark Banner (:standard8)]

(In reply to Geoff Lankow (:darktrojan) from comment #9)

• some name field to use in the UI

Assuming that's the table header, I think we might want the tooltip text as well. Conversations uses it to indicate that it is the sort field.

This would give an add-on the ability to put text in a cell and sort by that column. Am I missing any of the old abilities that are wanted?

I think you're just writing the back-end for experiment APIs to access, but I think at some stage before exposing this as a proper API, we'll need to think about performance. From what I recall, getting the text and the sort index are different calls in the column handler - so if we're going to do that via the proper API across to the extension process, that's going to double up the interprocess communication which will make things a lot worse. Hence it'd be nice to think about this before we do the proper API, and now might be the time whilst we're designing the initial parts.

Comment 11

[Geoff Lankow (:darktrojan)]

I can't think of any way to have this as a proper API and be performant. Unless… it wasn't done with callbacks, the data was computed ahead of time and cached by the parent process. That seems pretty ugly to me, but it could work.

I also don't see an API happening this side of 115. I'd like to build some common way of doing it before everybody comes up with their own hacky solution.

Comment 12

[Mark Banner (:standard8)]

Just to be clear, I wasn't advocating for an API before 115, just wanted to highlight some of the perf issues we might want to think about.

Comment 13

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1817682 - Functions for extensions to add custom column handlers. r=darktrojan

Comment 14

[Geoff Lankow (:darktrojan)]

Attachment: sample.zip

Here's a sample extension to try out my current implementation.

The implementation is a bit rough around the edges still, but it should be enough to show what I'm trying to do.

Comment 15

[John Bieling (:TbSync)]

Thanks for your work on this and sorry for my late feedback. I am able to get this working! You said you may have an updated version of the patch, if you have the time, could you publish it here as well?

Comment 16

[Mark Banner (:standard8)]

Adding tracking request, although it is obvious this won't get into the initial release. This is an important regression and blocker which will stop some add-ons being able to support 115.x as there is no way to work around it currently.

Comment 17

[Wayne Mery (:wsmwk)]

John, can you assign a priority?
Geoff, can you post the update - hopefully someone else can then iterate on it.
Thanks

Comment 18

[John Bieling (:TbSync)]

I am on it, but it is not as simple as I thought initially.

Comment 19

[Sven Giermann]

Any progress on this?
I still consider downgrading back to 102 rather than waiting for extensions to be available for 115...

Comment 20

[John Bieling (:TbSync)]

Attachment: Bug 1817682 - Allow to refresh only a single column. r=#thunderbird-front-end-reviewers

Depends on D179632

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ed5caa97f280
Functions for extensions to add custom column handlers. r=darktrojan
https://hg.mozilla.org/comm-central/rev/66f8a66081c7
Allow to refresh only a single column. r=aleca

Comment 22

[John Bieling (:TbSync)]

For add-on developers following this thread, the actual API is developed in Bug 1615801.

There is also still a pending group sort patch (Bug 1877390)

Comment 23

[John Bieling (:TbSync)]

Attachment: Bug 1817682 - [ESR115] Functions for extensions to add custom column handlers. r=#thunderbird-reviewers

This is a clone of https://phabricator.services.mozilla.com/D179632,
adjusted for Thunderbird ESR 115.

Comment 24

[John Bieling (:TbSync)]

Attachment: Bug 1817682 - [ESR115] Allow to refresh only a single column. r=#thunderbird-reviewers

This is a clone of https://phabricator.services.mozilla.com/D196361,
adjusted for Thunderbird ESR 115.

Depends on D204877

Comment 25

[John Bieling (:TbSync)]

Comment on attachment 9391697 [details]
Bug 1817682 - [ESR115] Functions for extensions to add custom column handlers. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): The new mail tab implementation introduced in Thunderbird 115.
User impact if declined: No add-on support for custom columns.
Testing completed (on c-c, etc.): Full beta cycle.
Risk to taking this patch (and alternatives if risky): This does touch many moving parts, but there have been no complaints during the last beta cycle.

Comment 26

[John Bieling (:TbSync)]

Comment on attachment 9391698 [details]
Bug 1817682 - [ESR115] Allow to refresh only a single column. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): The new mail tab implementation introduced in Thunderbird 115.
User impact if declined: No add-on support for custom columns.
Testing completed (on c-c, etc.): Full beta cycle.
Risk to taking this patch (and alternatives if risky): This does touch many moving parts, but there have been no complaints during the last beta cycle.

Comment 27

[Wayne Mery (:wsmwk)]

Comment on attachment 9391697 [details]
Bug 1817682 - [ESR115] Functions for extensions to add custom column handlers. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr115

Comment 28

[Wayne Mery (:wsmwk)]

Comment on attachment 9391698 [details]
Bug 1817682 - [ESR115] Allow to refresh only a single column. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr115

Comment 29

[Wayne Mery (:wsmwk)]

What can we suggest as a well defined list of steps/addons for 115 smoke testers? (https://addons.thunderbird.net/en-GB/thunderbird/search/?q=column is a long list)

• https://addons.thunderbird.net/en-GB/thunderbird/addon/no-message-pane-sort-by-mouse/ ?
• https://addons.thunderbird.net/en-GB/thunderbird/addon/xnotepp/ ?

These probably do not have 115 compatible versions yet:

• https://addons.thunderbird.net/en-GB/thunderbird/addon/full-address-column/
• https://addons.thunderbird.net/en-GB/thunderbird/addon/mailhops/
• https://addons.thunderbird.net/en-GB/thunderbird/addon/qnote/
• https://addons.thunderbird.net/en-GB/thunderbird/addon/thunvatar/

Comment 30

[Rob Lemley [:rjl]]

Thunderbird 115.10.0:
https://hg.mozilla.org/releases/comm-esr115/rev/6051d34851df

Comment 31

[Rob Lemley [:rjl]]

I missed one of the patches, but the other one is causing a compile error on c-esr115 so it will be backed out.
https://treeherder.mozilla.org/jobs?repo=comm-esr115&selectedTaskRun=crX0_V-iRWu1qMpGa3xKXA.0

Comment 32

[Rob Lemley [:rjl]]

Thunderbird 115.10.0:
https://hg.mozilla.org/releases/comm-esr115/rev/9ce9a95ec0b7

Comment 33

[Rob Lemley [:rjl]]

Thunderbird 115.10.0:
https://hg.mozilla.org/releases/comm-esr115/rev/f75c1f987586

Comment 34

[John Bieling (:TbSync)]

We now have two add-ons released for Thunderbird 115.10, which use the new custom column support:

• Full Address Column
• Show InOut

They can be used for smoke testing.