"Open all..." for attachments is available in File -> Attachments menu, but not attachments pane

RESOLVED FIXED in Thunderbird 5.0b1

Status

enhancement
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: squib, Assigned: squib)

Tracking

Trunk
Thunderbird 5.0b1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Under File -> Attachments, there's an "Open all..." menu item, but there isn't one in the attachment pane, either in the list itself, or as a menu item from the toolbar button. Here's a patch to add that.
Attachment #530228 - Flags: review?(bwinton)
Just a note of caution: I wouldn't use "Open All" on a closed attachment list where all information I have is "3 attachments" or so, not knowing which type of attachment it is and if it potentially disguises a virus...

It's probably ok to offer "Open All" when the attachment pane is open and the user has verified that it's safe to open them all at the same time.
Comment on attachment 530228 [details] [diff] [review]
Add "Open all..." to attachment list and toolbar button

Review of attachment 530228 [details] [diff] [review]:

First off, the patch failed to apply for me on trunk.  :(

So I manually fixed it up, I think, but you should probably look into that…

Secondly, I think we want tests for this, cause I'm like that.

Other than that, it looks good to me, so r=me with the fixup and some tests.
Attachment #530228 - Flags: review?(bwinton) → review+
So, you don't see this being a possible security hazard per comment #1,
or did I read Jim's patch wrong? (I didn't apply it.)
I agree that it's a potential security hazard, but not much more of one than the Attachments menu already is, and we are partially mitigating it by hiding the option behind a drop-down button.

Furthermore, forcing the user to open the attachment list doesn't guarantee that they'll look at all the attachments, much less look for things that might be viruses, it just adds an extra roadblock they need to go through to do what they want.

And who's to say that they didn't already open the attachment pane in a previous session, and now really just want to show Gramma the pictures of their grandkids?  ;)

So, to sum up, you're right about the security hazard, but I think it's minor enough that it shouldn't block the UI.

Thanks,
Blake.
(In reply to comment #4)
> I agree that it's a potential security hazard, but not much more of one than
> the Attachments menu already is, and we are partially mitigating it by hiding
> the option behind a drop-down button.

Wrong, in contrast to the drop-down menu File > Attachments also shows you the actual attachments with name and icon (unless someone changed that by now), thus it's easy to spot an ".scr" or something else that may look suspicious.

> Furthermore, forcing the user to open the attachment list doesn't guarantee
> that they'll look at all the attachments, much less look for things that might
> be viruses, it just adds an extra roadblock they need to go through to do what
> they want.

In that case it's their fault at least, at some point one has to accept that the user still wants to do something stupid though we've tried to prevent that.

> And who's to say that they didn't already open the attachment pane in a
> previous session, and now really just want to show Gramma the pictures of their
> grandkids?  ;)

That's the only point I'd accept, though one could keep track of that.
> (comment #4) it just adds an extra roadblock they need to go through to do what
> they want.

BTW: That's an interesting statement given that you denied removing that roadblock in bug 647036 as part of Thunderbird to start with...
(In reply to comment #5)
> (In reply to comment #4)
> > I agree that it's a potential security hazard, but not much more of one than
> > the Attachments menu already is, and we are partially mitigating it by hiding
> > the option behind a drop-down button.
> 
> Wrong, in contrast to the drop-down menu File > Attachments also shows you the
> actual attachments with name and icon (unless someone changed that by now),
> thus it's easy to spot an ".scr" or something else that may look suspicious.
> 

Yes, File >Attachments most definitely shows you the filenames and extension.
TB won't open executables but we should give some thought to providing the ability to "save all" attachments without first displaying them also.

Make it easier to show Grandma the pictures, but lets not encourage careless behaviors along the way.
I don't think it's actually possible to run an attached executable in Thunderbird without saving it first.
That's probably true for "real" executables which don't need any helper application to run. Something like a ".vbs" script may still work though.
It's hard for me to estimate any security implications of different file
types, but I nevertheless wouldn't open what I don't recognize, which means
I'd have to see it first to make that determination...

> Make it easier to show Grandma the pictures, but lets not encourage careless
> behaviors along the way.

That about sums it up.
Checked in: http://hg.mozilla.org/comm-central/rev/b69d9a4cc169

(Worst-case scenario, we back this out, but I wanted to get it in before string freeze since it has r+ anyway.)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
To be precise:
> (comment #2) so r=me with the fixup and some tests.

Thus, if Blake is picky he'd request this bug to be reopened for the tests or that you open a follow-up bug to take care of those - or that you back it out. ;-)

And, I still think it's encouraging careless behavior unless "Open All" is blocked at least for new messages before the attachments are made visible, but that logic also could be added after the string freeze...
squib, how soon do you think you can get the tests written?

Thanks,
Blake.
(In reply to comment #12)
> squib, how soon do you think you can get the tests written?

Probably this evening. I forgot to enable/disable the "Open all..." menuitem properly, so I'll be fixing that too. It should be straightforward, however.
You need to log in before you can comment on or make changes to this bug.