156368 - quicksearch fails if focus was in threadpane before search box

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Tuukka Tolvanen (sp3000)]

linux trunk cvs 2002-07-08

str:
1. select a newsgroup or mail message in threadpane
2. ctrl-shift-tab or click to focus quicksearch bar
3. type some characters

results:
Message is deselected in threadpane: blurred-selection disappears,
message-related mail buttons get disabled. Messages are not filtered.

expected:
Message stays selected, filter messages in threadpane according to search
string.

This is a fairly recent regression, two weeks max or so

bonus level:
4. hit backspace
-> quicksearch happens.

Reproducible: Always

Comment 1

[Tuukka Tolvanen (sp3000)]

Error: window.frames.messagepane has no properties
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 874

only on first pause in typing into qs box after moving focus from thread pane.

Comment 2

[Tuukka Tolvanen (sp3000)]

    gMessagePaneFrame = top.frames['messagepane'];
in function GetMessagePaneFrame() throws js strict warnings:

Warning: reference to undefined property top.frames.messagepane
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 787

also line 878 in the same file and line 452 in mailWindow.js give the warning.

Patch changes line 787 in GetMessagePaneFrame() to mimic GetMessagePane():
-    gMessagePaneFrame = document.getElementById("messagepane");
+    gMessagePaneFrame = document.getElementById("messagepane");
so GetMessagePaneFrame() works; and lines 874-875:
-		if (GetMessagePaneFrame().currentURI != "about:blank")
-		    GetMessagePaneFrame().loadURI("about:blank");
+		if (GetMessagePaneFrame().currentURI != "about:blank")
+		    GetMessagePaneFrame().loadURI("about:blank");
so ClearMessagePane() uses that.

Comment 3

[Tuukka Tolvanen (sp3000)]

Attachment: patch

Comment 4

[Tuukka Tolvanen (sp3000)]

er, s/878/874/ in comment 2, and the GetMessagePaneFrame().currentURI|loadUri
bits were window.frames["messagepane"].location, I'll have to learn to copypaste
right sometime :|

Comment 5

[laurel]

Just for clarification: trunk only, not regressed on commercial branch.

Comment 6

[Gilles Durys]

bug 59638 seems to be a wfm for several people (including myself),
but comes back to live with this patch applied.

Comment 7

[Tuukka Tolvanen (sp3000)]

perhaps 59638 works right because of ClearMessage() apparently having gotten
somehow broken at some point then...

Comment 8

[Tuukka Tolvanen (sp3000)]

Attachment: patch v2 incl 59638

The previous patch fixes bug 156556 (another symptom of ClearMessagePane() not
working) as well. This version also fixes bug 59638 that was "fixed" as a
symptom of same, and "regressed" as such by the previous patch version.

I moved the ClearMessagePane call up in FolderPaneSelectionChange, so now the
message pane is cleared when the folder changing begins, instead of after the
folder change when the message pane has possibly been updated with a message. 
In the original code the ClearMessagePane call would not have been done if
(gAccountCentralLoaded || gFakeAccountPageLoaded), but that would at least
theoretically lead to a race where
    1) select a message
    2) select an account
    3) select a folder
at step 3 the message pane is displayed before being cleared, so the message
might flash. I actually saw that happen, like, twice, ever. :)

If this looks right, please review this and obsolete attacment 90601, thanks

Comment 9

[Tuukka Tolvanen (sp3000)]

Attachment: patch v2.1 incl 59638

Whoops. Missed the return statement in the middle of
    FolderPaneSelectionChange, which makes it not do stuff on dismissing a
    context menu on a not-current folder/account, at least... Patch v2 did
    clear msg pane in this case. Moved the ClearMessagePane call to right
    before ChangeFolderByURI so the msg pane is cleared only when the folder
    pane selection is _really_ going to be changed.

Comment 10

[David G King]

Seeing this on Win98SE so OS should be changed to ALL

Comment 11

[Navin Gupta]

your patch looks good to me. I applied it and it worksforme. thx. 

Comment 12

[Navin Gupta]

Comment on attachment 90893 [details] [diff] [review]
patch v2.1 incl 59638

r=naving if you want I can
check it in for you.

Comment 13

[Navin Gupta]

cc bienvenu for sr

Comment 14

[David :Bienvenu]

Comment on attachment 90893 [details] [diff] [review]
patch v2.1 incl 59638

sr=bienvenu

Comment 15

[Tuukka Tolvanen (sp3000)]

yes naving pls checkin for me after a=. I mailed drivers for trunk approval.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 90893 [details] [diff] [review]
patch v2.1 incl 59638

a=roc+moz for TRUNK

Comment 17

[Navin Gupta]

fix checked on trunk. tuukka, thx for the patch. From laurel's comment not
needed on branch. 

Comment 18

[[e-mail]]

Somthing, possibly this patch, but possibly not, has caused two regressions:
1. Clicking in the message pane doesn't show the focus ring
2. Can't use the keyboard to scroll the message pane after Ctrl+Tab

Comment 19

[[e-mail]]

Attachment: Patch to fix message pane regressions

Comment 20

[Navin Gupta]

Comment on attachment 91419 [details] [diff] [review]
Patch to fix message pane regressions

r=naving. Have you checked
it doesn't cause anything else, unexpected ?

Please test because we don't want to break it again before shipping

Comment 21

[David :Bienvenu]

Comment on attachment 91419 [details] [diff] [review]
Patch to fix message pane regressions

sr=bienvenu, thx.

Comment 22

[[e-mail]]

Yes, I have tested this, I have not seen any other problems caused by my patch.
Older versions of Mozilla returned an [object Window] from GetMessagePaneFrame()
as does my patch whereas Tuukka's patch returned an [object XULElement].

Comment 23

[Asa Dotzler [:asa]]

Comment on attachment 91419 [details] [diff] [review]
Patch to fix message pane regressions

a=asa (on behalf of drivers) for checkin to 1.1

Comment 24

[[e-mail]]

Fix was checked in by timeless.

Comment 25

[Tuukka Tolvanen (sp3000)]

For the record, re comment 18, no, my patch (2002-07-12) didn't cause those
regressions, I suppose they appeared ~2002-07-09 along with bug 156556 (which my
patch fixed) and bug 156447 (which Neil's patch fixed). It was a sloppy fix
though (I didn't properly check the callers of GetMessagePaneFrame() were
getting what they expected), thanks for cleaning up :)

Comment 26

[laurel]

OK using aug 29 commercial trunk build: win98, linux rh6.2, mac OS 10.1