Open Bug 1817682 Opened 2 years ago Updated 1 year ago

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

Categories

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

Thunderbird 111
defect

Tracking

(thunderbird_esr115?)

Tracking Status
thunderbird_esr115 ? ---

People

(Reporter: standard8, Unassigned, NeedInfo)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [Supernova3p])

Attachments

(2 files)

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

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.

Blocks: 1615801
Whiteboard: [Supernova]
Version: unspecified → Thunderbird 111

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

(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);

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.

Flags: needinfo?(alessandro)

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.

Flags: needinfo?(alessandro)

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

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

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.

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

Whiteboard: [Supernova] → [Supernova3p]

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?

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

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.

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.

Attached file sample.zip (deleted) —

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.

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?

Flags: needinfo?(geoff)

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.

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

Attachment

General

Created:
Updated:
Size: