Closed Bug 178091 Opened 18 years ago Closed 15 years ago

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

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sspitzer, Assigned: neil)

Details

Attachments

(4 files, 6 obsolete files)

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?
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.
OS: Windows 2000 → All
Hardware: PC → All
Both Mail.app and 4.x used flat folder menus.
Attachment #105085 - Attachment is patch: false
Attachment #105085 - Attachment mime type: text/plain → image/png
Sounds good to me.
QA Contact: olgam → stephend
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.
Assignee: sspitzer → mscott
I've got this working for the advanced message search window.
Attached patch Work in progress (obsolete) — Splinter Review
Attached patch More progress (obsolete) — Splinter Review
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]
Attachment #152187 - Attachment is obsolete: true
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).
(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.
I think I can fix the mousewheel issue:
<handler event="DOMMouseScroll" preventdefault="true"/>
(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. :-)
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.
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. 
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?
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.
>* 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.
Do you need any help resolving some of these remaining open issues Neil?
(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.
Attached patch Further progress (obsolete) — Splinter Review
The only remaining issue is the -moz-user-focus: ignore; on the <treechildren>
which should be in xul.css as per bug 251465.
Attachment #152307 - Attachment is obsolete: true
Attached patch Ready (obsolete) — Splinter Review
This is now ready to replace the advanced search folder picker :-)
Attachment #153930 - Attachment is obsolete: true
Attachment #154442 - Flags: superreview?(bienvenu)
Attachment #154442 - Flags: review?(mscott)
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? 
Assignee: mscott → neil.parkwaycc.co.uk
Product: Browser → Seamonkey
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. 
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.
Attachment #186521 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #154442 - Attachment is obsolete: true
Attachment #154442 - Flags: superreview?(bienvenu)
Attachment #154442 - Flags: review?(mscott)
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.
Attachment #186521 - Flags: review?(neil.parkwaycc.co.uk) → review+
(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.
Attached patch updated patchSplinter Review
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.
Attachment #186521 - Attachment is obsolete: true
Attachment #186629 - Flags: superreview?(bienvenu)
Attachment #186629 - Flags: review+
Attachment #186629 - Flags: superreview?(bienvenu) → superreview+
Attachment #186629 - Flags: approval-aviary1.1a2?
Attachment #186629 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
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. 
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
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.
Attached patch Fix icons for thunderbird (obsolete) — Splinter Review
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.
Attachment #187025 - Flags: superreview?(bienvenu)
Attachment #187025 - Flags: review?(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...
Attachment #187025 - Flags: review?(bienvenu) → review?(mscott)
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.
(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...
Attachment #187025 - Flags: superreview?(bienvenu) → superreview+
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)
Attachment #187025 - Attachment is obsolete: true
Attachment #187025 - Flags: review?(mscott)
Moves shared folder pane tree children rules from mailWindow1.css to
folderPane.css so the new folder picker widget can pick them up.
Attachment #191633 - Attachment description: move shared folder tree rules into folderPane.css → [patch checked in] move shared folder tree rules into folderPane.css
Attached patch Page Down fixSplinter Review
I can't believe we missed this all along.
Attachment #219277 - Flags: review?(mscott)
Attachment #219277 - Flags: approval-branch-1.8.1?(mscott)
Attachment #219277 - Flags: review?(mscott)
Attachment #219277 - Flags: review+
Attachment #219277 - Flags: approval-branch-1.8.1?(mscott)
Attachment #219277 - Flags: approval-branch-1.8.1+
You need to log in before you can comment on or make changes to this bug.