Last Comment Bug 549802 - Reply button doesn't launch mail composition window.
: Reply button doesn't launch mail composition window.
Status: RESOLVED FIXED
: fixed-seamonkey2.0.4, regression
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Composition (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
:
Mentors:
Depends on:
Blocks: 546040
  Show dependency treegraph
 
Reported: 2010-03-02 20:45 PST by Victor Gigante-Hueber
Modified: 2010-03-16 13:18 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
add folderDisplay JS (1.38 KB, patch)
2010-03-04 12:55 PST, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review-
Details | Diff | Splinter Review
move folderDisplay JS [Checkin: comment 11] (2.61 KB, patch)
2010-03-15 09:17 PDT, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
neil: superreview+
kairo: approval‑seamonkey2.0.4+
Details | Diff | Splinter Review

Description Victor Gigante-Hueber 2010-03-02 20:45:01 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.3a3pre) Gecko/20100302 SeaMonkey/2.1a1pre
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.3a3pre) Gecko/20100302 SeaMonkey/2.1a1pre

I am using Seamonkey's Mail/News to access a Gmail IMAP account.  I have the message preview pane disabled, and I have Seamonkey set to open each new e-mail in a separate window.  Whenever I click on the reply button in the window of a message I want to reply to, nothing happens.  I can right-click on the message in the Thread Pane and click on "Reply to Sender Only", and I will get a window to compose my reply in.  But if I click the Reply button, Mail/News just sits there.

Reproducible: Always

Steps to Reproduce:
1. Open Mail/News.
2. Open a message.
3. Click on the Reply button.
Actual Results:  
Reply button registers a click by depressing, but no Reply window is launched.

Expected Results:  
Mail/News should have launched a message composition window with the sender's e-mail address in the "To:" field.
Comment 1 lenochod 2010-03-03 04:04:56 PST
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a2pre) Gecko/20100224 SeaMonkey/2.1a1pre

Error Console display:

Error: gFolderDisplay is not defined
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 1131
Comment 2 Serge Gautherie (:sgautherie) 2010-03-03 13:28:32 PST
(In reply to comment #1)

Likely culprit: bug 546040.
Comment 3 Jens Hatlak (:InvisibleSmiley) 2010-03-03 16:15:12 PST
(In reply to comment #2)
> (In reply to comment #1)
> 
> Likely culprit: bug 546040.

Yes.

I guess we need to add
<script type="application/javascript" src="chrome://messenger/content/folderDisplay.js"/>
to suite/mailnews/mailWindowOverlay.xul

TB includes it there, too:
<http://mxr.mozilla.org/comm-central/search?string=chrome%3A%2F%2Fmessenger%2Fcontent%2FfolderDisplay.js&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central>

They also have it in the search dialog but I don't know if it's needed; probably not for this, though. Does anyone know why we have it in messenger.xul and not messageWindow.xul like TB?
Comment 4 Jens Hatlak (:InvisibleSmiley) 2010-03-04 12:55:35 PST
Created attachment 430403 [details] [diff] [review]
add folderDisplay JS

This fixes it for me. Luckily the back-end functions called by gFolderDisplay already take care of stand alone mode windows, i.e. the actually displayed message is checked rather than the one currently selected in the thread pane:

gFolderDisplay.selectedMessage -> gDBView.hdrForFirstSelectedMessage -> GetKeyForFirstSelectedMessage:
    // If we don't have a tree selection we must be in stand alone mode...
<http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgDBView.cpp#7022>
Comment 5 neil@parkwaycc.co.uk 2010-03-04 14:12:40 PST
Comment on attachment 430403 [details] [diff] [review]
add folderDisplay JS

Remove it from messenger.xul, no?
Comment 6 Jens Hatlak (:InvisibleSmiley) 2010-03-04 14:55:14 PST
(In reply to comment #5)
> (From update of attachment 430403 [details] [diff] [review])
> Remove it from messenger.xul, no?

I don't know which implications that might have (I didn't introduce it in messenger.xul in the first place) so I'll let Karsten decide. Cf. comment 3: TB has it in 3 files, we (currently) have it only one. For example I don't know if we'd have to add the line to our messageWindow.xul if we removed it from messenger.xul.
Comment 7 neil@parkwaycc.co.uk 2010-03-07 13:37:13 PST
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 430403 [details] [diff] [review] [details])
> > Remove it from messenger.xul, no?
> TB has it in 3 files,
One too many, in fact.
> we (currently) have it only one. For example I don't know if we'd have to
> add the line to our messageWindow.xul if we removed it from messenger.xul.
But if you're adding it to mailWindowOverlay.xul then it will get used whenever mailWindowOverlay does, i.e. both messenger and messageWindow. Or you could ignore the overlay and just add it to messageWindow.xul directly.
Comment 8 Karsten Düsterloh 2010-03-14 15:47:34 PDT
Comment on attachment 430403 [details] [diff] [review]
add folderDisplay JS

>+<script type="application/javascript" src="chrome://messenger/content/folderDisplay.js"/>

If you add this here, there's no need to have it in messenger.xul, unless we had some code which used folderDisplay.js and ran before adding overlays. But that should be visible as errors on startup.

Just remove the script tag from messenger.xul.
Comment 9 Jens Hatlak (:InvisibleSmiley) 2010-03-15 09:17:27 PDT
Created attachment 432568 [details] [diff] [review]
move folderDisplay JS [Checkin: comment 11]

Bug 546040 depends on this and since I requested branch approval there I do it here, too, using approval‑seamonkey2.0.4 to be in sync with bug 546040. Any change (e.g. move to approval‑seamonkey2.0.5) to the flags there should be done here, too.
Comment 10 Karsten Düsterloh 2010-03-15 15:32:49 PDT
Comment on attachment 432568 [details] [diff] [review]
move folderDisplay JS [Checkin: comment 11]

Also a204=me here and in 546040 if Neil agrees.
Comment 11 Jens Hatlak (:InvisibleSmiley) 2010-03-16 10:16:34 PDT
Comment on attachment 432568 [details] [diff] [review]
move folderDisplay JS [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/2750fdd9a5f6
Comment 12 Jens Hatlak (:InvisibleSmiley) 2010-03-16 13:18:26 PDT
http://hg.mozilla.org/releases/comm-1.9.1/rev/2fa7744a093f

Note You need to log in before you can comment on or make changes to this bug.