Closed Bug 1634946 Opened 5 years ago Closed 4 years ago

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

Categories

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

Tracking

(thunderbird78 fixed)

RESOLVED FIXED
Thunderbird 79.0
Tracking Status
thunderbird78 --- fixed

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.

Summary: "Initially Show Attachment Pane" options not working, already broken in TB 68 ESR → "Initially Show Attachment Pane" option not working, already broken in TB 68 ESR

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

Priority: -- → P5

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.

Priority: P5 → P1
Regressed by: pgp
Summary: "Initially Show Attachment Pane" option not working, already broken in TB 68 ESR → "Initially Show Attachment Pane" option not working after Enigmail integration (also not working in TB 68 with Enigmail).
Attached patch 1634946.patch (deleted) — Splinter Review

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.

Attachment #9155441 - Flags: review?(mkmelin+mozilla)
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.
Attachment #9155441 - Flags: review?(mkmelin+mozilla) → review+
Assignee: nobody → jorgk-bmo
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 79.0
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.
Attachment #9155441 - Flags: approval-comm-beta?

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/6a2b0a6335ab
Re-instate 'Initially Show Attachment Pane' option. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Comment on attachment 9155441 [details] [diff] [review] 1634946.patch Approved for beta
Attachment #9155441 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: