Last Comment Bug 660882 - search window does not allow to open message from result list [currentHeaderData is not defined]
: search window does not allow to open message from result list [currentHeaderD...
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- blocker (vote)
: seamonkey2.4
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
: 663689 665842 669147 669215 669854 (view as bug list)
Depends on: 637835
Blocks: 671605
  Show dependency treegraph
 
Reported: 2011-05-31 10:09 PDT by Martin F.
Modified: 2011-09-25 16:19 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
patch [Checkin: comment 11] (4.32 KB, patch)
2011-05-31 13:55 PDT, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
neil: superreview+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
bugspam.Callek: approval‑seamonkey2.1-
Details | Diff | Splinter Review

Description Martin F. 2011-05-31 10:09:14 PDT
When performing a search via the search window (Tools -> Search Messages) it's impossible to open a message by double-clicking or selcting it and hitting the open button.

The error-message in the error-console differs:

Double click:
Fehler: currentHeaderData is not defined
Quelldatei: chrome://messenger/content/mailWindowOverlay.js
Zeile: 546

Open button:
Fehler: An error occurred executing the cmd_open command: [Exception... "'[JavaScript Error: "currentHeaderData is not defined" {file: "chrome://messenger/content/mailWindowOverlay.js" line: 546}]' when calling method: [nsIController::doCommand]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://global/content/globalOverlay.js :: goDoCommand :: line 96"  data: yes]
Quelldatei: chrome://global/content/globalOverlay.js
Zeile: 100

Confirmed for SM2.1RC and SM2.2pre
Comment 1 Jens Hatlak (:InvisibleSmiley) 2011-05-31 11:10:49 PDT
Reproducible with IMAP, POP3 and Local Folders, but not with News & Blogs.

Hmm, currentHeaderData is defined in/filled by msgHdrViewOverlay.js which is included by msgHdrViewOverlay.xul, which is included by mailWindowOverlay.xul.

I smell a "single message window"-only breakage.

I just hope I didn't regress this through fixing bug 637835.
Comment 2 Jens Hatlak (:InvisibleSmiley) 2011-05-31 11:29:24 PDT
The fact that it works (only) for feeds is that this breaks in function IsFeedItem (mailWindowOverlay.js) which returns early if the account is an RSS one.
Comment 3 Jens Hatlak (:InvisibleSmiley) 2011-05-31 11:49:18 PDT
Bah, last known good 2011-05-07, first bad 2011-05-08 -> probably bug 637835. :-(
Comment 4 Jens Hatlak (:InvisibleSmiley) 2011-05-31 13:54:07 PDT
OK, so it really was bug 637835 that regressed this:

GetFirstSelectedMessage (msgMail3PaneWindow.js), called from IsFeedItem (mailWindowOverlay.js), used to return null (from the catch, because gDBView was null so gDBView.URIForFirstSelectedMessage could not be accessed) so the currentHeaderData check in IsFeedItem was never reached.

Since the renaming of gSearchView to gDBView in bug 637835, gDBView is now defined, so GetFirstSelectedMessage now returns gDBView.URIForFirstSelectedMessage and the rest of IsFeedItem is checked.

In short, the IsFeedItem code contains a non-obvious assumption that if there is gDBView, there is currentHeaderData. This is bad.

The patch I'll attach in a minute gets rid of that code and the assumption.
Comment 5 Jens Hatlak (:InvisibleSmiley) 2011-05-31 13:55:28 PDT
Created attachment 536403 [details] [diff] [review]
patch [Checkin: comment 11]
Comment 6 Jens Hatlak (:InvisibleSmiley) 2011-05-31 14:26:28 PDT
Adding relnote keyword for now; if this doesn't make 2.1 final, then surely the first minor release (the patch applies against both comm-central and comm-2.0).
Comment 7 neil@parkwaycc.co.uk 2011-05-31 16:53:25 PDT
Comment on attachment 536403 [details] [diff] [review]
patch [Checkin: comment 11]

>-           'content-base' in currentHeaderData));
Presumably this is a port of the relevant parts of bug 474701 (in particular I notice comments 154-156 cover this issue.)
Comment 8 Jens Hatlak (:InvisibleSmiley) 2011-06-06 04:56:42 PDT
Comment on attachment 536403 [details] [diff] [review]
patch [Checkin: comment 11]

Karsten: ping for review (has sr=Neil)
Comment 9 Karsten Düsterloh 2011-06-07 16:25:42 PDT
Comment on attachment 536403 [details] [diff] [review]
patch [Checkin: comment 11]

>-  if (IsFeedItem() && GetFeedOpenHandler() == 2) {
>+  if (gFolderDisplay.selectedMessageIsFeed && GetFeedOpenHandler() == 2) {

Please fix the { position, while you're at it.
Comment 10 Jens Hatlak (:InvisibleSmiley) 2011-06-07 22:25:27 PDT
Comment on attachment 536403 [details] [diff] [review]
patch [Checkin: comment 11]

This is a bad regression (included in 2.1 final) which we should fix for any affected branch that has the potential to ship after 2.1 final.

I take it that the uplift to comm-beta has not happened yet. If I'm wrong, please understand this as requesting permission to land on comm-beta, too.
Comment 11 Jens Hatlak (:InvisibleSmiley) 2011-06-07 22:42:12 PDT
Comment on attachment 536403 [details] [diff] [review]
patch [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/aeae283c99ef
Comment 12 Justin Wood (:Callek) 2011-06-07 23:50:39 PDT
Comment on attachment 536403 [details] [diff] [review]
patch [Checkin: comment 11]

We did uplift to comm-beta already.
Comment 14 Jens Hatlak (:InvisibleSmiley) 2011-06-13 13:08:08 PDT
*** Bug 663689 has been marked as a duplicate of this bug. ***
Comment 15 Philip Chee 2011-06-21 07:05:55 PDT
*** Bug 665842 has been marked as a duplicate of this bug. ***
Comment 16 Michael Lueck 2011-06-25 04:11:54 PDT
Is the "Target Milestone: seamonkey2.2" true? We have to wait all of the way for 2.2 to get this one fixed? Or could it be snuck into the first security fix for the 2.1 series?
Comment 17 Ian Neal 2011-06-25 04:36:07 PDT
(In reply to comment #16)
> Is the "Target Milestone: seamonkey2.2" true? We have to wait all of the way
> for 2.2 to get this one fixed? Or could it be snuck into the first security
> fix for the 2.1 series?

2.2 is the first security fix for 2.1 and is expected out in the next few weeks.
Comment 18 Philip Chee 2011-07-04 11:10:05 PDT
*** Bug 669147 has been marked as a duplicate of this bug. ***
Comment 19 rsx11m 2011-07-04 20:32:45 PDT
*** Bug 669215 has been marked as a duplicate of this bug. ***
Comment 20 Philip Chee 2011-07-07 05:24:58 PDT
*** Bug 669854 has been marked as a duplicate of this bug. ***
Comment 21 Michael Lueck 2011-07-08 19:20:26 PDT
I just received 2.2 via UbuntuZilla this evening. Confirmed fixed with the official Linux build of 2.2. Thanks!
Comment 22 Philip Chee 2011-07-14 02:39:46 PDT
From: http://forums.mozillazine.org/viewtopic.php?p=11021099#p11021099

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++I have recently upgraded to SM 2.2. This bug is only PARTIALLY fixed. While it is now possible to open a particular message in the message search results by double clicking a given message, after ONE message window is opened, if ANOTHER message in the search results is double clicked WHILE the existing opened message window is STILL open, then the subsequent messages double clicked will FAIL to display in the already opened message window - the FIRST message will remain displayed, even though it's obvious that the opened message window refreshes. The only workaround at this moment in SM 2.2 is to always close the e-mail window opened on the message double clicked, then double click another e-mail. This is not the correct display action in a properly working SM mail client mail search window. I can't believe they did not check something as simple as this. If there's anyone that can re-update the status of this bug entry, it would be appreciated. Thank you.
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comment 23 Glutton.Vidal 2011-07-14 07:41:59 PDT
It's really true, that only one message can be opened/displayed during an extended search action.
Comment 24 John Elsworth 2011-07-14 09:13:48 PDT
(In reply to comment #22)
> From: http://forums.mozillazine.org/viewtopic.php?p=11021099#p11021099
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++I have
> recently upgraded to SM 2.2. This bug is only PARTIALLY fixed. While it is
> now possible to open a particular message in the message search results by
> double clicking a given message, after ONE message window is opened, if
> ANOTHER message in the search results is double clicked WHILE the existing
> opened message window is STILL open, then the subsequent messages double
> clicked will FAIL to display in the already opened message window - the
> FIRST message will remain displayed, even though it's obvious that the
> opened message window refreshes. The only workaround at this moment in SM
> 2.2 is to always close the e-mail window opened on the message double
> clicked, then double click another e-mail. This is not the correct display
> action in a properly working SM mail client mail search window. I can't
> believe they did not check something as simple as this. If there's anyone
> that can re-update the status of this bug entry, it would be appreciated.
> Thank you.
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

There was another workaround: click on the button at the bottom "Open Message Folder" the chosen message is highlighted and can be opened.
Comment 25 Jens Hatlak (:InvisibleSmiley) 2011-07-14 09:35:33 PDT
The actual workaround is to set mailnews.reuse_message_window to false.
Comment 26 Jens Hatlak (:InvisibleSmiley) 2011-07-14 10:59:28 PDT
(In reply to comment #22)
> From: http://forums.mozillazine.org/viewtopic.php?p=11021099#p11021099

Thanks for the info, moved to bug 671605.

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