178091 - implement Mail.app / 4.x Mac communicator like versions of the "move/copy" menus

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[(not reading, please use [e-mail] instead)]

implement Mail.app / 4.x Mac communicator like versions of the "move/copy" menus

sfraser will provide screen shot of 4.x mac and Mail.app

I've seen how it looks, and it would allow us to remove the "File Here" /
"Choose This Folder" thing we do.

jglick, comments?

Comment 1

[Simon Fraser [no longer active]]

Using hierarchical menus in popup menus makes them very hard to use, and forces
you to have hacks like the 'Choose this folder' item at the top of each submenu.
Things are much clearaer, and easier to use, if you flatten the entire
accounts/mailbox list into one menu, using indentation to indicate containment.

Comment 2

[Simon Fraser [no longer active]]

Attachment: Screenshot: how Mail.app on OS X does this

Both Mail.app and 4.x used flat folder menus.

Comment 3

[jglick]

Sounds good to me.

Comment 4

[Scott MacGregor]

reviving this bug. I think this would be a big usability win at hopefully a
pretty low cost.

cc'ing some possible volunteers who might be interested in trying to do this for
menu items that contain the user's account / folder hierarchies.

Comment 5

[[e-mail]]

I've got this working for the advanced message search window.

Comment 6

[[e-mail]]

Attachment: Work in progress

Comment 7

[[e-mail]]

Attachment: More progress

Fixes:
* The icons now work
* The dotted lines are now hidden
* Removed some event handlers by changing treechildren binding
* Clicking on the empty space at the bottom no longer throws an exception
Issues:
* Mousewheeling crashes in layout code
[unchecked null pointer]
* Clicking on scrollbarbuttons generates unwanted command events
[other mouse events are already blocked, but command got overlooked]
* Clicking in the tree generates unwanted focus events
[I need to investigate why this happens]

Comment 8

[Scott MacGregor]

I've been playing around with this latest patch too. Is the fact that selection
changes as you move the scrollbar up and down because of the issue you cited
where scrollbarbuttons generates unwanted command events? sounds like it
probably is. 

This may not be easy to do, but it is a bit weird to see all of the various
folder states effect this menu (i.e. if a server or folder has new mail I see a
biff indicator in the menu, if a folder has unread messages it is in bold, etc).

Comment 9

[[e-mail]]

(In reply to comment #8)
>I've been playing around with this latest patch too. Is the fact that
>selection changes as you move the scrollbar up and down because of the issue
>you cited where scrollbarbuttons generates unwanted command events? sounds
>like it probably is. 
Hmm... moving the scrollbar thumb shouldn't be an issue...

>This may not be easy to do, but it is a bit weird to see all of the various
>folder states effect this menu (i.e. if a server or folder has new mail I see
>a biff indicator in the menu, if a folder has unread messages it is in bold,
>etc).
My patch was based on bug 21344, which does need all of the folder states. I
then looked at what Outlook does and it in fact shows the bold and unread count.
Presumably it uses the same widget thoughout. Here however we have a bit of
flexibility, in that we can create slightly different bindings by changing the
properties applied to each cell, in this case by removing biffState, newMessages
 (mistyped as mewMessages anyway!) and hasUnreadMessages.

Comment 10

[[e-mail]]

I think I can fix the mousewheel issue:
<handler event="DOMMouseScroll" preventdefault="true"/>

Comment 11

[Stefan Borggraefe]

(In reply to comment #10)
> I think I can fix the mousewheel issue:
> <handler event="DOMMouseScroll" preventdefault="true"/>

Yes, I tested the "More progress" patch with this additional line and now the
mousewheel crasher is gone. :-)

Comment 12

[Scott MacGregor]

yeah that fixed the crash for me too. Are either of you seeing the problem where
moving the scroll bar up and down causes the highlight in the menu popup to move
up and down as well? 

i.e. if i drag the scroll bar up and down, the selection matches the position of
my cursor even though the cursor is over the scrollbar and not in the menu.

Comment 13

[Scott MacGregor]

Actually I can still trigger a crash using the scroll wheel if the cursor is
outside of the menu and the scroll bar. i.e. if the widget has focus, but i move
my mouse to the right of the scrollbars so it's not inside the widget or the
scrollbars, then use the scroll wheel, I crash:

nsMenuFrame::GetScrollableView

with aView being null. 

Comment 14

[Scott MacGregor]

Since I'm spamming the bug, I thought of another requirement for this widget. In
some cases (like the search window), the account should be selectable like it is
now. i.e. you can search on Local Folders or on a specific account.

In other places where we would want to use this widget, an account is not a
valid selection. You need to select a folder. For instance, if we used this
widget for the menu popup underneath the File toolbar button. Or the "Other"
menu list for an account for selecting a Templates/Sent/Drafts folder. Can we
add an attribute to the widget which specifies if a server is selectable then
leverage the noSelect-true cell property?

Comment 15

[[e-mail]]

I believe that the noselect flag is actually used to indicate folders that
cannot contain messages. What I thought was in that case of selecting an account
to store drafts it could automatically create the default folder, although that
doesn't help in the case of the File button; alternatively you could just ignore
clicks on accounts although I can see how it might be neater to make it so that
accounts don't even highlight when you mouse over them.

Comment 16

[[e-mail]]

>* Mousewheeling crashes in layout code
Still trying to understand scrolling code on this one.
>* Clicking on scrollbarbuttons generates unwanted command events
Fixed in another bug.
>* Clicking in the tree generates unwanted focus events
This is caused by the -moz-user-focus: style for <treechildren> being none,
rather than ignore. If I can persuade bryner that ignore is correct for all
unfocusable XUL elements, then the problem will go away. In the meantime, adding
-moz-user-focus: ignore; to the .foldersTreeChildren rule will make the <handler
event="mousedown" preventdefault="true"> unnecessary.
>* Hovering scrollbar moves the selection
I think this is actually caused by hovering the border.

Comment 17

[Scott MacGregor]

Do you need any help resolving some of these remaining open issues Neil?

Comment 18

[[e-mail]]

(In reply to comment #16)
>>* Mousewheeling crashes in layout code
>Still trying to understand scrolling code on this one.
I think rather than trying to catch bryner on irc I should just email him.
>>* Clicking in the tree generates unwanted focus events
>This is caused by the -moz-user-focus: style for <treechildren> being none,
>rather than ignore. If I can persuade bryner that ignore is correct for all
I filed a bug on this, but no review yet.
>>* Hovering scrollbar moves the selection
>I think this is actually caused by hovering the border.
Easiest fix is to do an if (event.target != this) check in the handler.

Comment 19

[[e-mail]]

Attachment: Further progress

The only remaining issue is the -moz-user-focus: ignore; on the <treechildren>
which should be in xul.css as per bug 251465.

Comment 20

[[e-mail]]

Attachment: Ready

This is now ready to replace the advanced search folder picker :-)

Comment 21

[Scott MacGregor]

Neil, I still see the selection box move with the position of my mouse as I move
the scrollbar thumb up and down the scroll box frame. Do you think that's a bug
or it should do that? 

Comment 22

[Scott MacGregor]

I recently re-discovered this bug again and tried Neil's latest patch. And my
problem with selection moving with the scroll bars is now gone, things are
working really well. I'll attach an updated patch against the trunk. 

Comment 23

[Scott MacGregor]

Attachment: updated patch to the trunk including changes for thunderbird

Neil, this is still your patch just updated to the trunk. Asking for an r again
just to confirm that you are still happy with the work that you did.

Comment 24

[[e-mail]]

Comment on attachment 186521 [details] [diff] [review]
updated patch to the trunk including changes for thunderbird

>+          <rule nc:CanSearchMessages="false"/>
>+          <rule>
These rules are no longer correct due to virtual folders.
In my tree I collapsed them into the following rule:
<rule nc:CanSearchMessages="true" nc:Virtual="false">

Somebody pointed out that I only provided basic keyboard navigation, there's no
typeaheadfind feature. Maybe I can implement that at a later date.

Comment 25

[[e-mail]]

(In reply to comment #24)
>there's no typeaheadfind feature.
By which I mean locating a folder by typing a unique prefix of its name.

Comment 26

[Scott MacGregor]

Attachment: updated patch

Updated patch with Neil's review comment. I've verified that virtual folders
don't show up in the search dialog menu list now. Good catch.

Moving Neil's r forward.

Comment 27

[Scott MacGregor]

Ok, this patch has been checked in! Woo hoo.

I'm going to close this bug out. Once the dust settles we can convert more
folder picker widgets to use this new widget. Right now the only consumer is the
search dialog folder picker. 

Comment 28

[Markus Faßbender]

It's a very nice view of my folders, but if you have a lot accounts with
subfolders, than it could be a little bit complex. 

It would be easier if you use the default folder icons in this new folder picker. 

So you will find your junk or trash folder very fast.

Comment 29

[[e-mail]]

Attachment: Fix icons for thunderbird

Unfortunately Thunderbird's icon CSS is in a different place. This is a quick
hack to make the icons show up for Thunderbird too. Or I could move the icons
back into the equivalent of the file that the Suite still uses.

Comment 30

[David :Bienvenu]

Comment on attachment 187025 [details] [diff] [review]
Fix icons for thunderbird

Scott's going to look at this and see why it makes a difference...

Comment 31

[Scott MacGregor]

Comment on attachment 187025 [details] [diff] [review]
Fix icons for thunderbird

Neil, I see that this change works by testing it, but I don't understand why it
works. When I look at qute's folderMenus.css file and compare it to classic's
(the suite) folderMenus.css file, they have the same rules (but with different
images of course).

They both have .folderMenuItem style rules for the various types of folders.

Comment 32

[[e-mail]]

(In reply to comment #31)
>They both have .folderMenuItem style rules for the various types of folders. 
Except that the new widget uses a different set of style rules...

Comment 33

[Scott MacGregor]

Comment on attachment 187025 [details] [diff] [review]
Fix icons for thunderbird

I'm going to move the folder pane tree children rules back to folderPane.css
instead for thunderbird (this is not an issue for seamonkey)

Comment 34

[Scott MacGregor]

Attachment: [patch checked in] move shared folder tree rules into folderPane.css

Moves shared folder pane tree children rules from mailWindow1.css to
folderPane.css so the new folder picker widget can pick them up.

Comment 35

[[e-mail]]

Attachment: Page Down fix

I can't believe we missed this all along.