122006 - Expanding (Imap / news server) account with keyboard doesn't check for new messages

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Jonas Jørgensen]

(Sorry if this is a dup - I searched a long time but couldn't find one)

When you expand the list of newsgroups on a server by clicking the twisty with
the mouse, Mozilla checks for new messages. That does not happen when expanding
the list by pressing the right arrow.

Comment 1

[[e-mail] (gone - use [e-mail] instead)]

Confirmed with build 2002-02-02-08 on Windows 2000.

Comment 2

[Jonas Jørgensen]

CC'ing aaronl on this major accessibility bug.

Comment 3

[Olga]

Wow, good test: right arrow expands the account AND checks for new messages,
left arrow collapse it!  (should)

Comment 4

[Harry Lu]

I have a patch for this bug. I will paste it here soon for review.

Comment 5

[Harry Lu]

Attachment: (Av1) Patch V1

Add a keydown listener for folderTree. Most of the code logic is from
FolderPaneDoubleClick().
Please r= and sr= this patch. Thanks!

Comment 6

[Jonas Jørgensen]

Comment on attachment 99795 [details] [diff] [review]
(Av1) Patch V1

Why use |switch| with only one |case|? Why not just use |if (event.keyCode ==
39|?

Comment 7

[Harry Lu]

I use "switch" here for the sake of further extending. I saw the same usage in
other codes.
Anyway, the basic logic is the same.  Surely I can change it to "if" if needed.
How about other people's thinking?

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Could the proposed patch be implemented ? We still have the see the bug in
2003032808 and this a major accessibility bug Which should be fixed (believe me,
I currently have a tendinitis and I am searching all means to mimimize my arm
movements!). I think this should be fixed for 1.4.

Comment 9

[Mike Cowperthwaite]

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

Comment 10

[Uri Dor]

confirmed in build 2004071408 on win2K

Comment 11

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20050104] (nightly) (W98SE)
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a5) Gecko/20041122] (release) (W98SE)

Bug still there.

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 99795 [details] [diff] [review]
(Av1) Patch V1

David:
Quite old (2002) patch,
which I could update if needed,
but I'd like to have a review/comment from you first.
Thanks.

Comment 13

[David :Bienvenu]

Comment on attachment 99795 [details] [diff] [review]
(Av1) Patch V1

+        var folderResource = GetFolderResource(folderTree, folderIndex);
+        var msgFolder = folderResource.QueryInterface(Components.interfaces.nsIMsgFolder);
+        var isServer = GetFolderAttribute(folderTree, folderResource, "IsServer");
+
+        if (isServer == "true")
+        {
+            if (!(folderTree.treeBoxObject.view.isContainerOpen(folderIndex)))
+            {
+                var server = msgFolder.server;
+                server.performExpand(msgWindow);
+            }
+        }

I think this can be re-arranged and simplified a bit:

if GetFolderAttribute(folderTree, folderResource, "IsServer")== "true" && !folderTree.treeBoxObject.view.isContainerOpen(folderIndex)))
{
  // no need to get this unless we're a server...
  var msgFolder == ...
  msgFolder.server.performExpand(msgWindow)

I wish there were some way to get notified when the tree is expanding, so we could react to that instead of trapping events that cause it to expand, but I don't know how to do that...

Comment 14

[Karsten Düsterloh]

Just saw this bug: I always thought this different behaviour between mouse action and key stroke is actually a feature and hence I'm using it as such. If I want to update from the server I use the mouse, if I don't want, I use the keyboard...

Comment 15

[Serge Gautherie (:sgautherie)]

(In reply to comment #14)
> Just saw this bug: I always thought this different behaviour between mouse
> action and key stroke is actually a feature and hence I'm using it as such.

It is believed that keyboard and mouse should behave the same way (by default).
Then, another bug could be to add a pref to make that behaviour +/- configurable...

Comment 16

[Arkady Belousov]

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

Comment 17

[Arkady Belousov]

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

Description for bug 359163 (Thunderbird 1.5.0.7): In left pane with accounts, place marker at collapsed News account. Click by mouse on '+' sign (to expand tree), and TB refreshes contents of branches from server. Now collapse tree back (by clicking by mouse on '-' sign or pressing Left arrow key) and expand by keyboard (Right arrow key) - tree expanded, but not refreshed from server.

Comment 18

[Serge Gautherie (:sgautherie)]

Attachment: (Av2-SM) <msgMail3PaneWindow.js>

Av1, with comment 13 suggestion(s),
and a litlle more.

Comment 19

[Serge Gautherie (:sgautherie)]

Comment on attachment 244592 [details] [diff] [review]
(Av2-SM) <msgMail3PaneWindow.js>

I tested this patch with
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20061102 SeaMonkey/1.1b] (nightly) (W98SE)

and I plan on requesting a1.8.1 for both SM and TB...

Comment 20

[David :Bienvenu]

Comment on attachment 244592 [details] [diff] [review]
(Av2-SM) <msgMail3PaneWindow.js>

thx for the patch...

Comment 21

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-TB) <msgMail3PaneWindow.js>

Av2-SM, ported/copied to TB,
with a few white space SM->TB syncs.
(Untested.)

Comment 22

[David :Bienvenu]

Comment on attachment 244592 [details] [diff] [review]
(Av2-SM) <msgMail3PaneWindow.js>

actually, since this is SM, a SM person should do one of the reviews...

Comment 23

[[e-mail]]

(In reply to comment #13)
>I wish there were some way to get notified when the tree is expanding, so we
>could react to that instead of trapping events that cause it to expand, but I
>don't know how to do that...
Sure, just use the folderObserver's onToggleOpenState callback.
[Note: that callback is supposed to have an index parameter]

Comment 24

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20061104 SeaMonkey/1.1b] (nightly) (W98SE)

(same code in TB)

While investigating Neil's suggestion (which seems to work just fine),
I came to test
[
function FolderPaneOnClick(event)
{

            var isServer = GetFolderAttribute(folderTree, folderResource, "IsServer");
            if (isServer == "true")

            else
            {
                var serverType = GetFolderAttribute(folderTree, folderResource, "ServerType");
                if (serverType == "imap")
]
and I can't trigger the "imap" block: the "isServer" test appears to apply to "imap" folders too !
Am I missing something ?
Should I "reorder" the blocks/code so that the "imap" one would apply when appropriate !?
Or is the "imap" block useless eventually ?

(This code is "blamed" to "1.151 <[e-mail]> 2001-08-14 22:07".)

Comment 25

[[e-mail]]

(In reply to comment #24)
>the "isServer" test appears to apply to "imap" folders too !
>Am I missing something ?
+pop account
  inbox
  drafts
  sent
  trash
+imap account
 +inbox <- imap code applies when this is opened
   bugmail
  sent
  trash
+local folders
  unsent message
  drafts
  sent
  trash

There are actually other ways of doing the tests in question.
1) folderTree.view.getLevel will return 0 for a server.
2) folderResource.QueryInterface(Components.interfaces.nsIMsgFolder).isServer
3) folderResource instanceof Components.interfaces.nsIMsgImapMailFolder

Comment 26

[David :Bienvenu]

Serge, are you going to submit a new patch as a result of Neil's tips?

Comment 27

[Serge Gautherie (:sgautherie)]

Attachment: (Av3-SM) <msgMail3PaneWindow.js>

Av2-SM, with comment 25 suggestion(s),
and a litlle more.

Would
|folderResource.QueryInterface(Components.interfaces.nsIMsgFolder).isServer|
(which I did not use) be "better" than
|folderTree.view.getLevel(index) == 0|
?

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20061110 SeaMonkey/1.1b] (nightly) (W98SE)

Comment 28

[[e-mail]]

Comment on attachment 245305 [details] [diff] [review]
(Av3-SM) <msgMail3PaneWindow.js>

>+        if (folderResource instanceof Components.interfaces.nsIMsgImapMailFolder)
>+        {
>+          // Imap4 message folder item.
>+
>+          folderResource.QueryInterface(Components.interfaces.nsIMsgImapMailFolder)
>+                        .performExpand(msgWindow);
instanceof calls QueryInterface so you don't need to do it again here.

>     else if (event.detail == 2) {
>+      if (elt.value != "twisty")
Can't you can combine these with an && ?

>-    var isServer = GetFolderAttribute(folderTree, folderResource, "IsServer");
>-    if (isServer == "true")
>+    if (folderTree.view.getLevel(folderIndex) == 0)
>     {
>-      if (!(folderTree.treeBoxObject.view.isContainerOpen(folderIndex)))
>+      if (!(folderTree.view.isContainerOpen(folderIndex)))
>       {
>-        var msgFolder = folderResource.QueryInterface(Components.interfaces.nsIMsgFolder);
>-        var server = msgFolder.server;
>-        server.performExpand(msgWindow);
>+        folderResource.QueryInterface(Components.interfaces.nsIMsgFolder)
>+                      .server.performExpand(msgWindow);
As far as I know, this should trigger onToggleOpenState anyway.

Comment 29

[Serge Gautherie (:sgautherie)]

Attachment: (Av4-SM) <msgMail3PaneWindow.js> [Checkin: Comment 32 & 34]

Av3-SM, with comment 28 suggestion(s).

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20061111 SeaMonkey/1.1b] (nightly) (W98SE)

An odd behaviour is that, quite often, using VK_RIGHT instead(!) of the twisty, on my Imap accounts (items),
the status bar displays "Looking for folders...", and it ends up like this.
(Could be my computer (...), the 1.8 branch, this code ???)

Another behaviour(s) which seems odd to me, but not this bug actually, is that when double clicking on a message folder to open it in a new window:
*the double-clicked folder on the first window gets selected/displayed :-(
*the folder on the second window is selected but not highlighted: gray instead of blue :-(

Comment 30

[[e-mail]]

Comment on attachment 245336 [details] [diff] [review]
(Av4-SM) <msgMail3PaneWindow.js>
[Checkin: Comment 32 & 34]

I see the "Looking for folders..." on an imap server when clicking the twisty, so I don't think that's your regression.

Comment 31

[Serge Gautherie (:sgautherie)]

Attachment: (Bv2-TB) <msgMail3PaneWindow.js> [Checkin: See comment 38]

Av4-SM, ported/copied to TB,
with a few white space SM->TB syncs.
(Untested.)

Comment 32

[Serge Gautherie (:sgautherie)]

Comment on attachment 245336 [details] [diff] [review]
(Av4-SM) <msgMail3PaneWindow.js>
[Checkin: Comment 32 & 34]


Checkin: {
2006-11-17 06:00	bugzilla%standard8.demon.co.uk 	mozilla/mailnews/base/resources/content/msgMail3PaneWindow.js 	1.296
}


'approval-seamonkey1.1=?':
Simple U.I. improvement, no risk.
(if not too late)

Comment 33

[Ian Neal]

Comment on attachment 245336 [details] [diff] [review]
(Av4-SM) <msgMail3PaneWindow.js>
[Checkin: Comment 32 & 34]

a=me for SM1.1

Comment 34

[Andrew Schultz]

Comment on attachment 245336 [details] [diff] [review]
(Av4-SM) <msgMail3PaneWindow.js>
[Checkin: Comment 32 & 34]

landed on the branch

Comment 35

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20061120 SeaMonkey/1.1b] (nightly) (W98SE)

V.Fixed on MOZILLA_1_8_BRANCH.

Comment 36

[David :Bienvenu]

Comment on attachment 245790 [details] [diff] [review]
(Bv2-TB) <msgMail3PaneWindow.js>
[Checkin: See comment 38]

thx, I'll try to land this today.

Comment 37

[David :Bienvenu]

fixed for TB trunk, thanks, Serge.

Comment 38

[Serge Gautherie (:sgautherie)]

Comment on attachment 245790 [details] [diff] [review]
(Bv2-TB) <msgMail3PaneWindow.js>
[Checkin: See comment 38]

For the record,
the checkin was done with a modified patch.

Comment 39

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1-TB) <msgMail3PaneWindow.js> [Checkin: See comment 49]

A few whitespace cleanups only:
after Bv2-TB modified Trunk checkin,
and before Dv1-TB-18 branch patch.

Comment 40

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1-TB-18) <msgMail3PaneWindow.js> [Checkin: Comment 45]

Port to (TB) 1.8 branch:
Bv2-TB + Cv1-TB
+ a few additional Trunk->Branch synchronizations
+ maybe a little more whitespace cleanups.

Comment 41

[David :Bienvenu]

Comment on attachment 246420 [details] [diff] [review]
(Dv1-TB-18) <msgMail3PaneWindow.js>
[Checkin: Comment 45]

thx for the patch

Comment 42

[Serge Gautherie (:sgautherie)]

Comment on attachment 246420 [details] [diff] [review]
(Dv1-TB-18) <msgMail3PaneWindow.js>
[Checkin: Comment 45]


'approval1.8.1.1=?': (Thunderbird only)
Trunk->Branch port. Low risk.

Comment 43

[Daniel Veditz [:dveditz]]

Comment on attachment 246420 [details] [diff] [review]
(Dv1-TB-18) <msgMail3PaneWindow.js>
[Checkin: Comment 45]

Way past the cut-off for non-blockers, moving request to 1.8.1.2

Comment 44

[Jay Patel [:jay]]

Comment on attachment 246420 [details] [diff] [review]
(Dv1-TB-18) <msgMail3PaneWindow.js>
[Checkin: Comment 45]

Approved for 1.8 branch, a=jay for drivers.  Let's get this in for Thunderbird 2.

Comment 45

[Serge Gautherie (:sgautherie)]

Comment on attachment 246420 [details] [diff] [review]
(Dv1-TB-18) <msgMail3PaneWindow.js>
[Checkin: Comment 45]


Checkin: {
2006-12-30 01:43	bugzilla%standard8.demon.co.uk 	mozilla/mail/base/content/msgMail3PaneWindow.js 	1.70.2.39 	MOZILLA_1_8_BRANCH
}
but the line 278 change, which was rewritten in v1.70.2.38 in the meantime.

Comment 46

[David :Bienvenu]

Comment on attachment 246419 [details] [diff] [review]
(Cv1-TB) <msgMail3PaneWindow.js>
[Checkin: See comment 49]

if I read this correctly, this patch is simply formatting - it doesn't really apply to this bug, does it?

Comment 47

[Serge Gautherie (:sgautherie)]

(In reply to comment #46)
> (From update of attachment 246419 [details] [diff] [review])
> if I read this correctly, this patch is simply formatting - it doesn't really

Yes.

> apply to this bug, does it?

A little at least: it's related to your previous "modified Bv2-TB" checkin
{
1.105	bienvenu%nventure.com	2006-11-21 09:14	 
}

Comment 48

[Carsten Book [:Tomcat]]

verified fixed for 1.8.1.2 on Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/20070125 Thunderbird/2.0b2pre Mnenhy/0.7.4.10002 ID:2007012504

Comment 49

[Serge Gautherie (:sgautherie)]

Comment on attachment 246419 [details] [diff] [review]
(Cv1-TB) <msgMail3PaneWindow.js>
[Checkin: See comment 49]


Checkin: {
2007-02-08 11:55	bugzilla%standard8.demon.co.uk 	mozilla/mail/base/content/msgMail3PaneWindow.js 	1.119
}

NB:
The |gFindBar.initFindBar();| line was removed in the meantime by
{
1.108	mozilla.mano%sent.com	2006-11-26 14:47	 	Bug 288254 - Findbar XBL widget. r=gavin,masayuki,bienvenu.
}

Comment 50

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/20070207 SeaMonkey/1.5a] (nightly) (W2Ksp4)

V.Fixed.