Menu "Message > Attachments" broken

RESOLVED FIXED in Thunderbird 66.0

Status

defect
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: jpmengual, Assigned: alta88)

Tracking

({regression})

Trunk
Thunderbird 66.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird65 fixed, thunderbird66 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 months ago

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

From a received message with an attach file:

  1. Press alt-m or f10 then right arrow eky until Message
  2. Press down key until "Attach file".

Actual results:

Here, the focus moves to the right menu instead of opening a submenu. Enter does notghing.

Expected results:

Right key or Enter should open a submenu displaying the file list, with themselves submenus to open, save, detach, etc.

Comment 1

4 months ago

I think something got lost in translation here. Sure, you can use keyboard navigation. The menu item is called "Attachments".

So here are some simpler STR:

  • Have a message with an attachment.
  • Tools > Attachments. This menu item stopped working.

Alice, can you please find the regression for us. Magnus/Arshad, do you feel responsible? Some de-XBL breakage? Already broken in TB 65 beta :-(

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(arshdkhn1)
Flags: needinfo?(alice0775)
Summary: Attach command unavailable → Menu "Message > Attachments" broken

Comment 2

4 months ago
  • Tools > Attachments. This menu item stopped working.

STR:
Select a message with an attachment in thread pane
Choose "Message" in Menu bar > "Attachments"

Actual results:
No submenu pops up

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=9c6c4038b3b5ab074c1d9c2bf91bae0a062adaab&tochange=89435f04afee556e91d8f17634cd377dff5ac8d2
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b4aeb99d1cb601e5a5288ca05630913fa8528a1c&tochange=6862624e24d005fb4f8fb07c6800d2acef1d287e

Flags: needinfo?(alice0775)

Comment 3

4 months ago

Oops, overlay removal caused this. Thank you so much as always, Alice!!

Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(arshdkhn1)

Comment 4

4 months ago

mail/base/content/mainNavigationToolbox.inc.xul
698 <menupopup id="attachmentMenuList" onpopupshowing="FillAttachmentListPopup(this);"/>

missing 1st arguments?
FillAttachmentListPopup( event , this)

Comment 5

4 months ago
Posted patch WIP.patch (obsolete) — Splinter Review

Hmm, yes, I found two call sites where the event argument was missing. Looking in the TB 60 code, it was missing there, too, but the overlay fixed it.

However, there's something else wrong, I marked the spot in the code.

Aceman, can you fix this for us please. Perhaps too much JS for Richard.

Flags: needinfo?(richard.marti) → needinfo?(acelists)
(Assignee)

Comment 6

4 months ago

the overlay changes also forgot the menuitems.. (noticed and fixed this in the attachments sec patch).

-              <menupopup id="attachmentMenuList" onpopupshowing="FillAttachmentListPopup(this);"/>
+              <menupopup id="attachmentMenuList"
+                         onpopupshowing="FillAttachmentListPopup(event, this);">
+                 <menuseparator/>
+                 <menuitem label="&openAllAttachmentsCmd.label;"
+                           accesskey="&openAllAttachmentsCmd.accesskey;"
+                           command="cmd_openAllAttachments" />
+                 <menuitem label="&saveAllAttachmentsCmd.label;"
+                           accesskey="&saveAllAttachmentsCmd.accesskey;"
+                           command="cmd_saveAllAttachments"/>
+                 <menuitem label="&detachAllAttachmentsCmd.label;"
+                           accesskey="&detachAllAttachmentsCmd.accesskey;"
+                           command="cmd_detachAllAttachments" />
+                 <menuitem label="&deleteAllAttachmentsCmd.label;"
+                           accesskey="&deleteAllAttachmentsCmd.accesskey;"
+                           command="cmd_deleteAllAttachments" />
+              </menupopup>
             </menu>
             <menuseparator id="messageAfterAttachmentMenuSeparator"/>

and appmenu

           <menupopup id="appmenu_attachmentMenuList"
-                     onpopupshowing="FillAttachmentListPopup(this);"/>
+                     onpopupshowing="FillAttachmentListPopup(event, this);">
+             <menuseparator/>
+             <menuitem label="&openAllAttachmentsCmd.label;"
+                       accesskey="&openAllAttachmentsCmd.accesskey;"
+                       command="cmd_openAllAttachments" />
+             <menuitem label="&saveAllAttachmentsCmd.label;"
+                       accesskey="&saveAllAttachmentsCmd.accesskey;"
+                       command="cmd_saveAllAttachments"/>
+             <menuitem label="&detachAllAttachmentsCmd.label;"
+                       accesskey="&detachAllAttachmentsCmd.accesskey;"
+                       command="cmd_detachAllAttachments" />
+             <menuitem label="&deleteAllAttachmentsCmd.label;"
+                       accesskey="&deleteAllAttachmentsCmd.accesskey;"
+                       command="cmd_deleteAllAttachments" />
+          </menupopup>
         </menu>
         <menuseparator class="appmenu-menuseparator"/>

Comment 7

3 months ago

Thanks, I'll submit this in your name.

Flags: needinfo?(acelists)

Updated

3 months ago
Assignee: nobody → alta88
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 8

3 months ago
Posted patch 1519481.patchSplinter Review

You have to rebase your other stuff now ;-)

Attachment #9036001 - Attachment is obsolete: true
Attachment #9036026 - Flags: review+

Comment 9

3 months ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bb6728bfe435
Restore Attachments submenu which got lost during overlay removal. r=jorgk

Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED

Updated

3 months ago
Target Milestone: --- → Thunderbird 66.0

Updated

3 months ago
Attachment #9036026 - Flags: approval-comm-beta+
(Reporter)

Comment 11

3 months ago

Hi,

I confirm the bug is fixed. Many thanks.

Regards

Blocks: 1446609
Keywords: regression
Depends on: 1520863
You need to log in before you can comment on or make changes to this bug.