Closed Bug 182121 Opened 17 years ago Closed 12 years ago

Can't open file attached from compose window

Categories

(MailNews Core :: Composition, enhancement)

x86
All
enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mfedyk, Assigned: bugzilla)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 7 obsolete files)

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.
Blocks: 44135
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)
Severity: major → enhancement
OS: Windows NT → All
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. ***
Attached patch "fix" for this bug (obsolete) — Splinter Review
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.
Attachment #117276 - Flags: superreview?(sspitzer)
Attachment #117276 - Flags: review?(ssu)
reassign to ssu
Assignee: ducarroz → 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.
Attachment #117276 - Flags: superreview?(sspitzer)
Attachment #117276 - Flags: superreview-
Attachment #117276 - Flags: review?(ssu)
Attachment #117276 - Flags: review-
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.
Attachment #117276 - Attachment is obsolete: true
*** 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. ***
Product: MailNews → Core
*** 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
Attachment #216079 - Flags: review-
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());
Blocks: TB2SM
Attached patch Backport from Thunderbird v1.1 (obsolete) — Splinter Review
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.
Assignee: ssu0262 → neuos
Attachment #117623 - Attachment is obsolete: true
Attachment #216079 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #262147 - Flags: review?(iann_bugzilla)
Attachment #262147 - Flags: superreview?(bienvenu)
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.
Attachment #262147 - Flags: review?(iann_bugzilla) → review+
Attached patch Backport from Thunderbird v1.2 (obsolete) — Splinter Review
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.
Attachment #262147 - Attachment is obsolete: true
Attachment #266642 - Flags: superreview?(bienvenu)
Attachment #266642 - Flags: review+
Attachment #262147 - Flags: superreview?(bienvenu)
Attachment #266642 - Flags: superreview?(bienvenu) → superreview?(neil)
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">
Attachment #266642 - Flags: superreview?(neil) → superreview-
Attached patch Backport from Thunderbird v1.3 (obsolete) — Splinter Review
(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...
Attachment #266642 - Attachment is obsolete: true
Attachment #267305 - Flags: superreview?(neil)
Attachment #267305 - Flags: review+
Attached patch Backport from Thunderbird v2.0 (obsolete) — Splinter Review
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.
Attachment #267305 - Attachment is obsolete: true
Attachment #267322 - Flags: superreview?(neil)
Attachment #267322 - Flags: review?(iann_bugzilla)
Attachment #267305 - Flags: superreview?(neil)
(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.
Attachment #267322 - Attachment is obsolete: true
Attachment #267322 - Flags: superreview?(neil)
Attachment #267322 - Flags: review?(iann_bugzilla)
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.
Depends on: 349309
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.
Attachment #274188 - Flags: superreview?(neil)
Attachment #274188 - Flags: review?(iann_bugzilla)
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.
Attachment #274188 - Flags: superreview?(neil) → superreview+
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.
Attachment #274188 - Flags: review?(iann_bugzilla) → review+
Thanks for the reviews, and sorry about the patch (I'll run diff from the top level directory next time).
Keywords: checkin-needed
Here is the corrected patch for checkin.
Whiteboard: [checkin needed]
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [checkin needed]
No longer blocks: TB2SM
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.