1906833 - getComposeDetails() fails when the toolbar button "Encrypt" is not shown in the toolbar

Status: RESOLVED FIXED

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

Description

[Alexander Bergmann]

In the compose window, when the toolbar button "Encrypt" was manually removed by the user and is not shown in the toolbar anymore, then messenger.compose.getComposeDetails() fails with this error:

composeWindow.document.getElementById(...) is null ext-compose.js:487:6
    getComposeDetails chrome://messenger/content/parent/ext-compose.js:487
    getComposeDetails chrome://messenger/content/parent/ext-compose.js:1826
    AsyncFunctionNext self-hosted:804

This is a regression by Bug 1732669, which introduced support for encryption properties as part of the compose details. It affects Thunderbird 128; Thunderbird 115 is NOT affected.

Comment 1

[John Bieling (:TbSync)]

Attachment: Bug 1906833 - Fix compose.getComposeDetails() if the "encryption" button has been removed. r=#thunderbird-reviewers

The purpose of checking the actual button state is to make sure to
report enabled encryption, even if no encryption technology is available
anymore due to an identity change or other actions performed by the
user. In this case, we still want to report enabled encryption, if the
user had it enabled. This is an invalid state and the message will not
be send (because encryption is not possible).

To fix the reported bug, we fallback to a state flag, if the button has
been removed. That state flag should be identical to the button state,
but Thundebird's security engineer explicitly wanted to check the button
state, so we keep doing that, if the button is available

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5e079d121fa0
Fix compose.getComposeDetails() if the "encryption" button has been removed. r=mkmelin

Comment 4

[John Bieling (:TbSync)]

Comment on attachment 9411883 [details]
Bug 1906833 - Fix compose.getComposeDetails() if the "encryption" button has been removed. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): 1732669
User impact if declined: Major regression, a standard API call causes the compose window to freeze if the encryption button has been removed
Testing completed (on c-c, etc.): one day, no negative impacts
Risk to taking this patch (and alternatives if risky): Low

Comment 5

[John Bieling (:TbSync)]

Comment on attachment 9411883 [details]
Bug 1906833 - Fix compose.getComposeDetails() if the "encryption" button has been removed. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): 1732669
User impact if declined: Major regression, a standard API call causes the compose window to freeze if the encryption button has been removed
Testing completed (on c-c, etc.): one day, no negative impacts
Risk to taking this patch (and alternatives if risky): Low

Comment 6

[John Bieling (:TbSync)]

Comment on attachment 9411883 [details]
Bug 1906833 - Fix compose.getComposeDetails() if the "encryption" button has been removed. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): 1732669
User impact if declined: Major regression, a standard API call causes the compose window to freeze if the encryption button has been removed
Testing completed (on c-c, etc.): one day, no negative impacts
Risk to taking this patch (and alternatives if risky): Low

Comment 7

[Wayne Mery (:wsmwk)]

Comment on attachment 9411883 [details]
Bug 1906833 - Fix compose.getComposeDetails() if the "encryption" button has been removed. r=#thunderbird-reviewers

[Triage Comment]
Approved for beta
Approved for esr128
APproved for release

Comment 8

[Daniel Darnell [:dandarnell]]

Thunderbird 128.0.1esr:
https://hg.mozilla.org/releases/comm-esr128/rev/4c3dd649fd43

Comment 9

[Daniel Darnell [:dandarnell]]

Thunderbird 129.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/d3718cf907d9

Comment 10

[Jonathan Kamens]

Why isn't this fix listed in the Thunderbird 128.0.1 release notes? Ref: https://www.thunderbird.net/en-US/thunderbird/128.0.1esr/releasenotes/

More broadly, if we (add-on developers) can't rely on the release notes reliably telling us what's fixed in each release, where can we go to find out what fixes are included?

Comment 11

[Wayne Mery (:wsmwk)]

It is customary for many years that backend add-on API fixes are not in release notes, but rather communicated in other ways - because release notes are oriented to users.

To follow add-on bug activity there is "watch this component" in the "Categories" section at the top of this bug page.

Comment 12

[Jonathan Kamens]

Add-on developers are users and customers of Thunderbird too. The TB team should be proactively communicating bugs and fixes that impact us just as they proactively communicate bugs and fixes that impact end users.

The difference between "for many years" and now is that now TB has (or claims to have) a stable, supported add-ons API. One of the whole points of that API is that add-on developers aren't supposed to have to follow the internal workings of the TB team to consume it. The add-ons API is a product. Documenting bugs and fixes in a product is a best practice.

Telling people to follow a firehose of emails from bugzilla just so that we can get notified when a fix is released is unreasonable.

Finally, I do not understand why the tracking flags for esr128 and 129 were just changed from "fixed" to "affected." Has this been fixed in esr128 and 129 or not?

This very question is proof of what I am saying about documenting these fixes properly. It is unreasonable to expect add-on maintainers to be able to interpret tracking flags in bug tickets, especially when the folks maintaining TB apparently don't even always set them correctly!

Comment 13

[John Bieling (:TbSync)]

ESR 128.0.1 was released while I was on PTO and I was not yet able to properly document these fixes. We document API changes here:
https://webextension-api.thunderbird.net/en/128-esr-mv2/changes/esr128.html

I will update that page very soon with changes in 128.0.1.

Comment 14

[Jonathan Kamens]

(In reply to John Bieling (:TbSync) from comment #13)

ESR 128.0.1 was released while I was on PTO and I was not yet able to properly document these fixes. We document API changes here:
https://webextension-api.thunderbird.net/en/128-esr-mv2/changes/esr128.html

I will update that page very soon with changes in 128.0.1.

Thank you. Looking at that page now, I don't see any indication—perhaps I missed it—that bug fixes impacting the add-ons API are documented there. Just to be clear, are you saying it's your intention to document such fixes there moving forward? Or are they already documented there and I just missed it?

Comment 15

[Alexander Bergmann]

I am looking from time to time at the open and closed bugs in this component:
Product: Thunderbird - Component: Add-Ons: Extensions API

And at the pushlogs of Thunderbird 115 / 128 / Beta / Daily, e.g.:
https://hg.mozilla.org/releases/comm-esr115/pushloghtml
https://hg.mozilla.org/releases/comm-esr128/pushloghtml

Comment 16

[John Bieling (:TbSync)]

(In reply to Jonathan Kamens from comment #14)

(In reply to John Bieling (:TbSync) from comment #13)

ESR 128.0.1 was released while I was on PTO and I was not yet able to properly document these fixes. We document API changes here:
https://webextension-api.thunderbird.net/en/128-esr-mv2/changes/esr128.html

I will update that page very soon with changes in 128.0.1.

Thank you. Looking at that page now, I don't see any indication—perhaps I missed it—that bug fixes impacting the add-ons API are documented there. Just to be clear, are you saying it's your intention to document such fixes there moving forward? Or are they already documented there and I just missed it?

Some references, for example "Officially support data: urls and blob: urls for menu icons" are bug fixes, but I never included the bug number itself in the text. I once had added a link to all bugs which landed in a release in the past, for example here at the bottom:

https://web.archive.org/web/20221207143629/https://webextension-api.thunderbird.net/en/stable/changes/beta91.html

Based on your feedback, I will bring that back.

Comment 17

[John Bieling (:TbSync)]

(In reply to Jonathan Kamens from comment #14)

(In reply to John Bieling (:TbSync) from comment #13)

ESR 128.0.1 was released while I was on PTO and I was not yet able to properly document these fixes. We document API changes here:
https://webextension-api.thunderbird.net/en/128-esr-mv2/changes/esr128.html

I will update that page very soon with changes in 128.0.1.

Thank you. Looking at that page now, I don't see any indication—perhaps I missed it—that bug fixes impacting the add-ons API are documented there. Just to be clear, are you saying it's your intention to document such fixes there moving forward? Or are they already documented there and I just missed it?

They are now linked in the "Fixed defects" sections:
https://webextension-api.thunderbird.net/en/128-esr-mv2/changes/128.html

Comment 18

[Daniel Darnell [:dandarnell]]

Comment on attachment 9411883 [details]
Bug 1906833 - Fix compose.getComposeDetails() if the "encryption" button has been removed. r=#thunderbird-reviewers

Removing approval-comm-release+ to keep this bug out of the comm-release uplift queue, since it's already been merged.