User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.1) Gecko/20020826 Build Identifier: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.1) Gecko/20020826 While in the process of switching my users to mozilla from QuickMail (with a brief excursion to Netscape 4.7) I have several asking me why they can't double click on the file they have attached to be able to check it. Reproducible: Always Steps to Reproduce: 1. Open a compose window 2. Attach a file 3. Try to double-click on the attachment Actual Results: No responce Expected Results: Open the attachment (go directly to the file where it is currently located. No copying to a temp directory) with the program associated to that file type.

bug 120132 and bug 139721 are about the same as this one and both were resolved as duplicates of bug 44135.

No, bug 44135 is to view attachments that mozilla can decode (such as gif or jpeg images) in the message pane. This bug on the other hand is only asking mozilla to launch the associated application for the attchment (determined by mime type or OS environment) when you double click on it. Two different requests, but on the same general topic.

I would also like this feature to be added. I fiddled around with the source code yesterday but couldn't get it to happen, but maybe when I understand the code a little more... By the way, I think this bug should be an enhancement rather than major. And it's also Linux as well as Windows NT - it's probably All, but I don't know for sure.

Agreed, marking enhancement, all (I also see it on linux)

Would be great to have this implemented. Really useful feature for people that are sending highly sensitive and important attachments.

*** Bug 146842 has been marked as a duplicate of this bug. ***

*** Bug 196427 has been marked as a duplicate of this bug. ***

*** Bug 176369 has been marked as a duplicate of this bug. ***

*** Bug 197282 has been marked as a duplicate of this bug. ***

This is as far as I've gotten with this.

What does that patch do? It opens the attachment in a new browser window (window.open(blah)). It opens a new browser window even if the file will be handled externally or saved. That's not good. The browser window that gets opened doesn't have any decoration like a url bar and buttons and all. No security checks are done. I don't know what security checks need to be done, but other loaders do security checks (see contentAreaUtils.js). So how do i figure out whether the url will be handled internally or kicked to an external application? I guess nsIMIMEService will kind of tell me that. Is that the best way to find out? And having figured that out, how do I actually kick it to an external application? Using the nsIExternalAppService or whatever it was called seemed complicated, but I guess I could go for that. I guess there are other things like using entities instead of just "Open" for the context menu label, and maybe hooking this same code to double-clicks on the attachment. I'll continue working on that.

reassign to ssu

Comment on attachment 117276 [details] [diff] [review] "fix" for this bug i don't think you want to review this patch. the point of my comment was to say: this patch is a work in progress. but if ssu wants to actually work on it, i'll be very curious to know what a good fix looks like

Comment on attachment 117276 [details] [diff] [review] "fix" for this bug this patch isn't ready for review yet.

This is better than the last in a number of ways. Attachments open when double-clicked on in addition to the context menu item. the attachment label and accesskey are specified in the dtd. The new window pops up with browser chrome. Basically, the only problem is that the new window _always_ pops up, even when the external app menu is about to come up. I'm trying to get it to work the same way chatzilla works, which is to have a link with target="_content", but it seems that the compose window already has some content, in particular the mail text, whereas chatzilla doesn't. so that may end up not working at all. Anyway, the point is, this is better than nothing.

*** Bug 199145 has been marked as a duplicate of this bug. ***

*** Bug 200442 has been marked as a duplicate of this bug. ***

Hey guys, what's the status of this bug? I just had another user ask me about this (our previous sucky email program (quickmail) supported this, and my users are used to it after using it for a while). Any chance this patch could be cleaned up and made ready for 1.4b? + var attachment = attachmentList[0].attachment; + var link = document.createElementNS("http://www.w3.org/1999/xhtml","html:a"); + link.setAttribute("target","_new"); + link.setAttribute("href",attachment.url); + attachmentList[0].appendChild(link); So does this mean opening an attachment will hit http://www.w3.org/1999/xhtml or are we resetting the uri too fast for that? <window id="msgcomposeWindow" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" xmlns:nc="http://home.netscape.com/NC-rdf#" + xmlns:html="http://www.w3.org/1999/xhtml" And what does this do?

*** Bug 235526 has been marked as a duplicate of this bug. ***

*** Bug 237872 has been marked as a duplicate of this bug. ***

*** Bug 246406 has been marked as a duplicate of this bug. ***

*** Bug 257305 has been marked as a duplicate of this bug. ***

*** Bug 331500 has been marked as a duplicate of this bug. ***

This patch is basically a direct backport from Thunderbird code-wise. I haven't tested it yet on a fresh build, but by expanding my .jar files and making the changes in this patch on my live version of seamonkey, everything works as it should. I'll post an update comment after I've tried rebuilding from scratch with the patch to make sure everything gets applied properly.

(In reply to comment #24) > Created an attachment (id=216079) [edit] > ...I'll post an update comment after I've tried rebuilding > from scratch with the patch to make sure everything gets applied properly. > Yes, it seems to have applied the changes fine after a rebuild.

(In reply to comment #24) > Backport from Thunderbird -- applies against Seamonkey 1.0 source tree According to http://www.mozilla.org/projects/seamonkey/project-areas.html, you could/should request a review from Ian Neal (iann_bugzilla@arlen.demon.co.uk).

Comment on attachment 216079 [details] [diff] [review] Backport from Thunderbird -- applies against Seamonkey 1.0 source tree >diff -uNr mozilla.orig/mailnews/compose/resources/content/MsgComposeCommands.js mozilla/mailnews/compose/resources/content/MsgComposeCommands.js For next patch could you provide more context, preferably use pud8 >+++ mozilla/mailnews/compose/resources/content/MsgComposeCommands.js 2006-03-23 22:51:57.000000000 -0500 >@@ -516,10 +519,11 @@ > case "cmd_print" : DoCommandPrint(); break; > > //Edit Menu >- case "cmd_delete" : if (MessageHasSelectedAttachments()) RemoveSelectedAttachment(); break; >+ case "cmd_delete" : if (MessageGetNumSelectedAttachments()) RemoveSelectedAttachment(); break; > case "cmd_selectAll" : if (MessageHasAttachments()) SelectAllAttachments(); break; > case "cmd_account" : MsgAccountManager(null); break; > case "cmd_preferences" : DoCommandPreferences(); break; >+ case "cmd_openAttachment" : if (MessageGetNumSelectedAttachments() == 1) OpenSelectedAttachment(); break; Please make sure "break;" in added lines is inline with the one in cmd_selectAll >@@ -2486,13 +2491,28 @@ > return false; > } > >-function MessageHasSelectedAttachments() >+/* This is the old Seamonkey code: >+* function MessageHasSelectedAttachments() >+* { >+* var bucketList = document.getElementById("attachmentBucket"); >+ >+* if (bucketList) >+* return (MessageHasAttachments() && bucketList.selectedItems && bucketList.selectedItems.length); >+* return false; >+* } >+*/ >+ >+/* >+* This is Thunderbird code, backported here: >+*/ Do not comment out code, just remove it, cvs will store any history that is needed. >+function MessageGetNumSelectedAttachments() > { > var bucketList = document.getElementById("attachmentBucket"); > > if (bucketList) >- return (MessageHasAttachments() && bucketList.selectedItems && bucketList.selectedItems.length); >- return false; >+ return bucketList.selectedItems.length; >+ else >+ return 0; > } Do not return from an else, just return 0; at the end of the function. >@@ -2578,6 +2598,96 @@ > return element ? element.childNodes.length : 0; > } > >+function OpenSelectedAttachment() >+{ >+ var child; >+ var bucket = document.getElementById("attachmentBucket"); >+ if (bucket.selectedItems.length == 1) >+ { >+ var attachmentUrl = bucket.getSelectedItem(0).attachment.url; >+ >+ var messagePrefix = /^mailbox-message:|^imap-message:|^news-message:/i; >+ if (messagePrefix.test(attachmentUrl)) >+ { >+ // we must be dealing with a forwarded attachment, treat this special >+ var messenger = Components.classes["@mozilla.org/messenger;1"].createInstance(); >+ messenger = messenger.QueryInterface(Components.interfaces.nsIMessenger); >+ var msgHdr = messenger.messageServiceFromURI(attachmentUrl).messageURIToMsgHdr(attachmentUrl); Use the new msgHdrFromURI instead in the above line. >+ if (msgHdr) >+ { >+ var folderUri = msgHdr.folder.folderURL; >+ window.openDialog("chrome://messenger/content/messageWindow.xul", "_blank", "all,chrome,dialog=no,status,toolbar", >+ attachmentUrl, folderUri, null ); >+ } >+ } >+ else >+ { >+ // turn the url into a nsIURL object then open it >+ >+ var url = gIOService.newURI(attachmentUrl, null, null); >+ url = url.QueryInterface( Components.interfaces.nsIURL ); Do not put spaces between brackets and what is inside them, you probably do not need to include the above line anyway. >+ getInterface: function(iid) >+ { >+ if (iid.equals(Components.interfaces.nsIDOMWindowInternal)) >+ return window; >+ else >+ return this.QueryInterface(iid); >+ }, Again do not return from an else, just do this return at the end of the function. Once you have generated a new patch request a review through the flags and not via email. http://www.mozilla.org/hacking/code-review-faq.html

The current patch also generates the following message in the JS console: Error: [Exception... "'Component does not have requested interface' when calling method: [nsIInterfaceRequestor::getInterface]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCommands.js :: OpenSelectedAttachment :: line 2611" data: no] Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 2611 Line 2611 is: uriLoader.openURI(channel, true, new nsAttachmentOpener());

Here is an updated patch that fixes the bitrot and addresses Ian's comments in comment 27. Comment 28 affects Thunderbird as well, so I have filed bug 377993 to cover that.

Comment on attachment 262147 [details] [diff] [review] Backport from Thunderbird v1.1 >Index: MsgComposeCommands.js >=================================================================== >+function OpenSelectedAttachment() >+{ >+ var child; child seems to be an unused variable. >+ // turn the url into a nsIURL object then open it >+ >+ var url = gIOService.newURI(attachmentUrl, null, null); >+ url = url.QueryInterface(Components.interfaces.nsIURL); I'm not sure you need to QI url.

Thanks for the review, Ian. Here is an updated patch that addresses the review comments. Carrying over the r+ from v1.1 of the patch.

Comment on attachment 266642 [details] [diff] [review] Backport from Thunderbird v1.2 The trick with loading a link into content should work, and would be better than using a URI loader. See extensions/help/resources/content/help.js (it actually uses a separate frame for techincal reasons). I've continued the review in case you don't want to look in to that route just yet. >- return MessageHasSelectedAttachments(); >+ return MessageGetNumSelectedAttachments(); Should use != or > 0 to make the intent clear. >+ case "cmd_delete" : if (MessageGetNumSelectedAttachments()) RemoveSelectedAttachment(); break; Same again. > function updateEditItems() > { > goUpdateCommand("cmd_pasteNoFormatting"); > goUpdateCommand("cmd_pasteQuote"); > goUpdateCommand("cmd_delete"); > goUpdateCommand("cmd_selectAll"); >+ goUpdateCommand("cmd_openAttachment"); > goUpdateCommand("cmd_find"); > goUpdateCommand("cmd_findNext"); > goUpdateCommand("cmd_findPrev"); > } Is this really in the edit menu? >+ // we must be dealing with a forwarded attachment, treat this special Nit: specially >+ var messenger = Components.classes["@mozilla.org/messenger;1"].createInstance(); >+ messenger = messenger.QueryInterface(Components.interfaces.nsIMessenger); Nit: use createInstance(Components.interfaces.nsIMessenger); directly >+ var url = gIOService.newURI(attachmentUrl, null, null); >+ >+ if (url) >+ { >+ var channel = gIOService.newChannelFromURI(url); Nit: these methods probably throw on failure, so no point null-checking them. >+ QueryInterface: function(iid) Nit: aIID >+ onStartURIOpen: function(uri) >+ { >+ return; >+ }, >+ >+ doContent: function(contentType, isContentPreferred, request, contentHandler) >+ { >+ return; >+ }, >+ >+ isPreferred: function(contentType, desiredContentType) >+ { >+ return; >+ }, Nit: All these are supposed to be boolean so should also return false; >+ getInterface: function(iid) Nit: aIID again. >+ { >+ if (iid.equals(Components.interfaces.nsIDOMWindowInternal)) >+ return window; >+ return this.QueryInterface(iid); >+ }, >+ >+ loadCookie: null, >+ parentContentListener: null >+} >+ > function DetermineHTMLAction(convertible) > { > var obj; > if (! gMsgCompose.composeHTML) > { > try { > obj = new Object; > gMsgCompose.CheckAndPopulateRecipients(true, false, obj); >@@ -2928,16 +3019,18 @@ function AttachmentBucketClicked(event) > { > event.currentTarget.focus(); > > if (event.button != 0) > return; > > if (event.originalTarget.localName == "listboxbody") > goDoCommand('cmd_attachFile'); >+ else if (event.originalTarget.localName == "listitem" && event.detail == 2) >+ OpenSelectedAttachment(); > } > > var attachmentBucketObserver = { > > canHandleMultipleItems: true, > > onDrop: function (aEvent, aData, aDragSession) > { >Index: messengercompose.xul >=================================================================== >RCS file: /cvsroot/mozilla/mailnews/compose/resources/content/messengercompose.xul,v >retrieving revision 1.289 >diff -u -p -8 -r1.289 messengercompose.xul >--- messengercompose.xul 18 Jan 2007 23:20:34 -0000 1.289 >+++ messengercompose.xul 18 Apr 2007 18:50:47 -0000 >@@ -139,16 +139,17 @@ > > <!-- Edit Menu --> > <!--command id="cmd_pasteQuote"/ DO NOT INCLUDE THOSE COMMANDS ELSE THE EDIT MENU WILL BE BROKEN! --> > <!--command id="cmd_find"/--> > <!--command id="cmd_findNext"/--> > <command id="cmd_rewrap" oncommand="goDoCommand('cmd_rewrap')"/> > <command id="cmd_delete"/> > <command id="cmd_selectAll"/> >+ <command id="cmd_openAttachment" oncommand="goDoCommand('cmd_openAttachment')"/> > <command id="cmd_account" oncommand="goDoCommand('cmd_account')"/> > > <!-- View Menu --> > <command id="cmd_showComposeToolbar" oncommand="goDoCommand('cmd_showComposeToolbar')"/> > <command id="cmd_showFormatToolbar" oncommand="goDoCommand('cmd_showFormatToolbar')"/> > <command id="toggleSidebar"/> > > <!-- Options Menu --> >@@ -227,16 +228,17 @@ > <menuitem command="cmd_pasteNoFormatting"/> > <menuitem label="&pasteQuote.label;" accesskey="&pasteQuote.accesskey;" command="cmd_pasteQuote"/> > <menuitem label="&delete.label;" accesskey="&delete.accesskey;" command="cmd_delete"/> > <menuseparator/> > <menuitem label="&selectAll.label;" accesskey="&selectAll.accesskey;" command="cmd_selectAll"/> > </popup> > > <popup id="msgComposeAttachmentContext" onpopupshowing="updateEditItems();"> >+ <menuitem label="&openAttachment.label;" accesskey="&openAttachment.accesskey;" command="cmd_openAttachment"/> > <menuitem label="&delete.label;" accesskey="&delete.accesskey;" command="cmd_delete"/> > <menuitem label="&selectAll.label;" accesskey="&selectAll.accesskey;" command="cmd_selectAll"/> > <menuseparator/> > <menuitem label="&attachFile.label;" accesskey="&attachFile.accesskey;" command="cmd_attachFile"/> > <menuitem label="&attachPage.label;" accesskey="&attachPage.accesskey;" command="cmd_attachPage"/> > </popup> > > <toolbox class="toolbox-top" id="headers-box"> >Index: messengercompose.dtd >=================================================================== >RCS file: /cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/compose/messengercompose.dtd,v >retrieving revision 1.89 >diff -u -p -8 -r1.89 messengercompose.dtd >--- messengercompose.dtd 31 Mar 2007 22:09:11 -0000 1.89 >+++ messengercompose.dtd 18 Apr 2007 18:53:06 -0000 >@@ -209,16 +209,19 @@ > <!ENTITY cut.label "Cut"> > <!ENTITY cut.accesskey "t"> > <!ENTITY copy.label "Copy"> > <!ENTITY copy.accesskey "C"> > <!ENTITY paste.label "Paste"> > <!ENTITY paste.accesskey "P"> > <!ENTITY pasteQuote.label "Paste As Quotation"> > <!ENTITY pasteQuote.accesskey "Q"> >+ >+<!ENTITY openAttachment.label "Open"> >+<!ENTITY openAttachment.accesskey "O"> > <!ENTITY delete.label "Delete"> > <!ENTITY delete.accesskey "D"> > <!ENTITY selectAll.label "Select All"> > <!ENTITY selectAll.accesskey "A"> > <!ENTITY attachFile.label "Attach File(s)..."> > <!ENTITY attachFile.accesskey "F"> > <!ENTITY attachPage.label "Attach Web Page..."> > <!ENTITY attachPage.accesskey "W">

(In reply to comment #32) > Should use != or > 0 to make the intent clear. Fixed in the updated patch. > Same again. Fixed. > Is this really in the edit menu? The new "Open" item only shows up in the context menu for the attachment bucket, but that function is used for both the context menu and the Edit menu. > Nit: specially > Nit: use createInstance(Components.interfaces.nsIMessenger); directly > Nit: these methods probably throw on failure, so no point null-checking them. > Nit: aIID > Nit: All these are supposed to be boolean so should also return false; > Nit: aIID again. All fixed. > The trick with loading a link into content should work, and would be better > than using a URI loader. See extensions/help/resources/content/help.js (it > actually uses a separate frame for techincal reasons). I've continued the > review in case you don't want to look in to that route just yet. Good idea, but I ran into a problem when trying to implement that. I tried replacing lines 2692-2697 with this: var browserWin = getTopWin(); if (!browserWin) window.openDialog(getBrowserURL(), "_blank", "chrome,all,dialog=no", attachmentUrl); else { browserWin.loadURI(attachmentUrl); browserWin.content.focus(); } The code displays the attachments properly, but the problem is that with focus() there, the browser will still get the focus when SeaMonkey can't handle the attachment and pops up a content handling dialog, which could potentially confuse users. But without focus() there, the attachment loads in the background if SeaMonkey can handle it, which is far more confusing...

Sorry for the bugspam, but after further testing, I realized that when using the URI loader, opening the attachment would fail when there was no Navigator window open. Instead, I switched to the code that I posted in comment #33 and added some code to solve the focus problem. Re-requesting review due to the significant code change.

(In reply to comment #33) >>Is this really in the edit menu? >The new "Open" item only shows up in the context menu for the attachment >bucket, but that function is used for both the context menu and the Edit menu. Oh, you mean updateEditItems() - yes, I overlooked that. >>See extensions/help/resources/content/help.js >I tried replacing lines 2692-2697 with this: > > var browserWin = getTopWin(); I don't see getTopWin() used in extensions/help/resources/content/help.js ...

Comment on attachment 267322 [details] [diff] [review] Backport from Thunderbird v2.0 (In reply to comment #35) > I don't see getTopWin() used in extensions/help/resources/content/help.js ... No wonder I was having trouble getting this to work... I misunderstood and thought that you just meant to use the same general idea as help.js, but I see what you mean now. I'll upload an updated patch soon.

Well, the code from help.js works, but I'm running into some of the same problems as before. When an attachment that SeaMonkey can handle is opened, Navigator doesn't get focus when loading it. Additionally, when Navigator is not open and an attachment is opened, nothing happens. However, when SeaMonkey can't handle the attachment, everything works fine. Here is the code I used: var editorElement = GetCurrentEditorElement(); if (editorElement) { const loadFlags = Components.interfaces.nsIWebNavigation.LOAD_FLAGS_IS_LINK; try { editorElement.webNavigation.loadURI(attachmentUrl, loadFlags, null, null, null); } catch (e) {} } Since we don't have helpExternal here, I used editorElement instead (based on line 1393 of MsgComposeCommands.js).

(In reply to comment #37) >When an attachment that SeaMonkey can handle is opened, Navigator doesn't get focus when loading it. Your code snippet seemed to work for me. >Additionally, when Navigator is not open and an attachment is opened, nothing happens. This is a known bug in nsBrowserContentHandler.js, see bug 349309.

(In reply to comment #38) > (In reply to comment #37) > >When an attachment that SeaMonkey can handle is opened, Navigator doesn't get focus when loading it. > Your code snippet seemed to work for me. After further testing, it actually looks like Navigator is getting the focus, but then the Compose window is getting the focus back immediately afterward. This is easier to see if Navigator is minimized before opening the attachment. I only tested this on WinXP so far, so maybe this is OS-specific behavior? > This is a known bug in nsBrowserContentHandler.js, see bug 349309. Thanks, I'll mark that as a blocker.

Now that the patch for bug 349309 has been checked in, the code that I posted in comment #37 works properly, and all focus issues have been resolved. Here is the full patch.

Comment on attachment 274188 [details] [diff] [review] Backport from Thunderbird v3.0 Note: this patch is corrupt, since not all of the files are in the same directory, so you need to apply it in two halves.

Comment on attachment 274188 [details] [diff] [review] Backport from Thunderbird v3.0 r=me though in future don't hand merge two diffs just diff from a directory high enough to be common to all files.

Thanks for the reviews, and sorry about the patch (I'll run diff from the top level directory next time).

Here is the corrected patch for checkin.

Checking in mozilla/mailnews/compose/resources/content/MsgComposeCommands.js; /cvsroot/mozilla/mailnews/compose/resources/content/MsgComposeCommands.js,v <-- MsgComposeCommands.js new revision: 1.400; previous revision: 1.399 done Checking in mozilla/mailnews/compose/resources/content/messengercompose.xul; /cvsroot/mozilla/mailnews/compose/resources/content/messengercompose.xul,v <-- messengercompose.xul new revision: 1.290; previous revision: 1.289 done Checking in mozilla/suite/locales/en-US/chrome/mailnews/compose/messengercompose.dtd; /cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/compose/messengercompose.dtd,v <-- messengercompose.dtd new revision: 1.90; previous revision: 1.89 done