799451 - Can't remove attachment with keyboard in forwarded e-mail

Status: VERIFIED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Onno Ekker [:nONoNonO UTC+1]]

Sometimes I need to forward e-mails (inline), but I have to strip the attachment. So I select the message, press Ctrl+L to forward the message. Then in the Compose window I press Alt+M to go to the attachments bucket, press Space to select the attachment and press Del to delete it, but nothing happens.

I can delete the attachment by going to the menu Edit -> Remove attachment (Del).

I see the following error in the Error Console:
Timestamp: 9-10-2012 13:35:47
Error: TypeError: boundTarget is null
Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js
Line: 4175

It doesn't matter whether I'm in safe-mode or not.

Comment 1

[Ludovic Hirlimann [:Usul]]

Onno can you find the regression range ?

Comment 2

[Onno Ekker [:nONoNonO UTC+1]]

This stopped working between Thunderbird 11.0.1 and Thunderbird 12.0.1.
I'll have to dig deeper to narrow this range...

Comment 3

[Onno Ekker [:nONoNonO UTC+1]]

I've tested this with Thunderbird Daily / comm-central:
20120114030033 works
20120115030024 fails

I'm not sure how to go further from this, but from browsing hg around this date I see the following change is suspect, although there are other changes a bit later for backing out the change again:
http://hg.mozilla.org/comm-central/rev/25f87dd2453f

Comment 4

[Onno Ekker [:nONoNonO UTC+1]]

Next change is http://hg.mozilla.org/comm-central/rev/74aaf5c6fa48 - Jim Porter — Backout changeset 25f87dd2453f due to test bustage in test-attachment.js

Comment 5

[Jim Porter (:squib)]

This WFM on a fairly recent nightly.

Comment 6

[Onno Ekker [:nONoNonO UTC+1]]

Strange. I just tried with Daily 20121105030251 and it fails... I've used a fresh profile and tried both with composition in HTML as in plain text and both fail with the same error message, only with linenumber 4272.
I've tried both with Windows 7 32 bit and Windows XP and it fails on both OS-es.

Comment 7

[Onno Ekker [:nONoNonO UTC+1]]

The bug not only occurs when I press Alt+M, but also when I use Tab to go to the first attachment and press space to select it and then press Del. When I use Tab to go to the attachment, there is no error message in the error console.

When I select the message with Alt+M and space, press Shift+Tab to go to the Subject and press Tab again to go back to the AttachmentBucket, removing the attachment by pressing Del *does* work...

Comment 8

[Magnus Melin [:mkmelin]]

Yeah this is from bug 526998  - and 100% reproducible for me on linux.

Apparently due to a bug in listbox.xml code earlier there was an ugly workaround 

let enabled = bucketList && bucketList.getRowCount() && (bucketList == top.document.commandDispatcher.focusedElement);

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

What seems to happen is:
cmd_delete is one of the commands that is updated when focus changes (since it is used elsewhere too).
When entering the attachments area using keyboard (Alt+M) the attachmentBucket gets focus,
 -> command updating, but at that point there's no selection so cmd_delete is
disabled. When an item gets selected that doesn't cause the commands to get
updated, so cmd_deleted is always disabled.

There was also an exception for keyboard "clicks", since then the attachmentBucket is the event's original target, and that isn't anonymous, so doesn't have a bindingparent.

Comment 10

[Magnus Melin [:mkmelin]]

(And I don't think there was a bug in listbox.xml like earlier stated.)

Comment 11

[Jim Porter (:squib)]

Comment on attachment 690075 [details] [diff] [review]
proposed fix

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

I've never been able to reproduce this, even though I'm on Linux too, so I'll just trust you that this fixes it. The code *looks* sane, anyway.

Comment 12

[Magnus Melin [:mkmelin]]

http://hg.mozilla.org/comm-central/rev/7b9efef47391 -> FIXED

Comment 13

[Thomas D. (:thomas8)]

Would this be safe to land for versions earlier than TB20, esp esr17 for corporate environs?

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 690075 [details] [diff] [review]
proposed fix

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

This should be save to take on esr.

On a related note i don't know where the Alt+M shortcut to select the attachment bucket is coming from, but it seems undocumented?

Comment 15

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #14)
> On a related note i don't know where the Alt+M shortcut to select the
> attachment bucket is coming from, but it seems undocumented?

It comes from the attachment pane caption "1 attach_m_ent", which is only found in attachment pane of /composition/. So it's an *access key* (not a keyboard shortcut), and we generally do not document access keys because they are always visible in the primary UI itself. On some OS, you have to press Alt to see the access keys underlined in the UI.

Comment 16

[Mark Banner (:standard8)]

Comment on attachment 690075 [details] [diff] [review]
proposed fix

[Triage Comment]
Given the age of when this was introduced, we'll let the patch soak during the 19 beta cycle and take it into 17.0.2 (or whatever it is equivalent to when gecko 19 is released).

So we'll take it onto aurora now before the merge, so it'll be ready for the next beta.

Comment 17

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/a9f7d45e4d34

Comment 18

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr17/rev/bb0e6b6c5db8