1634946 - "Initially Show Attachment Pane" option not working after Enigmail integration (also not working in TB 68 with Enigmail).

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect, P1)

Description

[Jorg K (CEST = GMT+2)]

If you right-click on the attachment pane header, you get and option "Initially Show Attachment Pane". If selected, new compose windows should start with the attachment pane already open. That worked for a while after the feature was implemented. It seems to have gone broken, just like opening and closing an empty area broke, bug 1613284.

Comment 1

[Magnus Melin [:mkmelin]]

Since apparently nobody missed it, we could probably do without.

Comment 2

[Jorg K (CEST = GMT+2)]

Enigmail integration broke this:

If the preferences is set, we see this call stack:
=== toggleAttachmentPane show
== JS stack> chrome://messenger/content/messengercompose/MsgComposeCommands.js (6524)
== JS stack> chrome://messenger/content/messengercompose/MsgComposeCommands.js (3631)
== JS stack> chrome://messenger/content/messengercompose/MsgComposeCommands.js (3912)
== JS stack> chrome://messenger/content/messengercompose/messengercompose.xhtml(1)
coming from https://searchfox.org/comm-central/rev/024cb64ede43e4cfca243bf69e75c75a31d39adc/mail/components/compose/content/MsgComposeCommands.js#3631

Shortly after we get:
=== toggleAttachmentPane hide
== JS stack> chrome://messenger/content/messengercompose/MsgComposeCommands.js (6524)
== JS stack> chrome://messenger/content/messengercompose/MsgComposeCommands.js (6144)
== JS stack> chrome://messenger/content/messengercompose/MsgComposeCommands.js (6105)
== JS stack> chrome://openpgp/content/ui/enigmailMsgComposeOverlay.js (794)
== JS stack> chrome://openpgp/content/ui/enigmailMsgComposeOverlay.js (226)
coming from:
https://searchfox.org/comm-central/rev/024cb64ede43e4cfca243bf69e75c75a31d39adc/mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js#794

This is a regression and the statement

Since apparently nobody missed it, we could probably do without.

is wrong. It still works in TB 68 as long as Enigmail isn't installed.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1634946.patch

I'm really not pleased that the Enigmail code has just been committed to the source tree without a proper review by a Compose peer. It doesn't take much digging to find more issues :-(

I'm also not pleased that defects get assigned incorrect priorities without taking the time to at least look at the issue, and yes, my fault, I had messed up the description. Regression that are visible in the UI within 30 seconds of use of course need to be fixed, even if they only affect a minority of users. Attachment handling is clearly an enterprise feature and it is well imaginable that this option is used by some.

Magnus, please assign a resource to give the Enigmail compose code a thorough review.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 9155441 [details] [diff] [review]
1634946.patch

Review of attachment 9155441 [details] [diff] [review]:
-----------------------------------------------------------------

We'll clean up enigmail code eventually, but there are many moving pieces so hard to do atm.

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 9155441 [details] [diff] [review]
1634946.patch

[Approval Request Comment]
Regression caused by (bug #): Enigmail integration
User impact if declined: Visible regression within 30 seconds of using new version
Testing completed (on c-c, etc.): Yes, manually.
Risk to taking this patch (and alternatives if risky): Not risky, basically a one-liner checking a pref.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6a2b0a6335ab
Re-instate 'Initially Show Attachment Pane' option. r=mkmelin

Comment 7

[Wayne Mery (:wsmwk)]

Comment on attachment 9155441 [details] [diff] [review]
1634946.patch

Approved for beta

Comment 8

[Geoff Lankow (:darktrojan)]

Thunderbird 78.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/e149840af8ce