Last Comment Bug 454829 - summary page for collapsed threads and multiple selections
: summary page for collapsed threads and multiple selections
Status: ASSIGNED
[andreasn to tweak css][test plan com...
: dev-doc-needed
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
: -- normal with 4 votes (vote)
: Thunderbird 3.0b3
Assigned To: Andreas Nilsson (:andreasn)
:
:
Mentors:
: 495649 (view as bug list)
Depends on: 478167 499155 506933 510551 476103 479890 495304 495642 495665 495691 495859 495965 496248 496318 496646 496884 497891 498227 502387 502839 519689 524913 570406
Blocks: 327828 737347 448288 474523 496793
  Show dependency treegraph
 
Reported: 2008-09-11 10:42 PDT by Bryan Clark (DevTools PM) [:clarkbw]
Modified: 2016-06-17 05:53 PDT (History)
39 users (show)
ludovic: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP (16.21 KB, patch)
2009-01-27 00:49 PST, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v2 (9.33 KB, patch)
2009-01-28 00:11 PST, David Ascher (:davida)
no flags Details | Diff | Splinter Review
screenshot (110.38 KB, image/png)
2009-01-28 00:12 PST, David Ascher (:davida)
no flags Details
jquery patch (403.64 KB, patch)
2009-02-23 13:40 PST, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v3 (29.35 KB, application/octet-stream)
2009-02-23 13:44 PST, David Ascher (:davida)
no flags Details
WIP v4 (48.15 KB, patch)
2009-02-23 14:06 PST, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v5 (51.14 KB, patch)
2009-02-23 14:14 PST, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v6 (51.72 KB, patch)
2009-02-23 16:05 PST, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v7 (57.40 KB, patch)
2009-05-01 16:53 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v8 (38.31 KB, patch)
2009-05-04 09:46 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v9 (51.48 KB, patch)
2009-05-04 17:03 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
updated screenshot (103.30 KB, image/png)
2009-05-04 17:06 PDT, David Ascher (:davida)
no flags Details
WIP v10 (57.27 KB, patch)
2009-05-05 18:05 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
this includes my patch for handling selection, and tweaks davida's patch a bit, including fixing the selection issue (127.18 KB, patch)
2009-05-11 12:53 PDT, David :Bienvenu
no flags Details | Diff | Splinter Review
diff for interdiff (145.24 KB, patch)
2009-05-11 14:12 PDT, David :Bienvenu
no flags Details | Diff | Splinter Review
WIP v11 (56.29 KB, patch)
2009-05-13 12:16 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
WIP v12 (56.83 KB, patch)
2009-05-13 13:30 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
patch for review v1 (57.12 KB, patch)
2009-05-21 12:13 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
patch for review v2 (57.17 KB, patch)
2009-05-21 13:55 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
patch for review v3 (56.29 KB, patch)
2009-05-21 16:56 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
patch for review v4 (56.30 KB, patch)
2009-05-21 18:18 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
patch for review v5 (56.66 KB, patch)
2009-05-22 14:02 PDT, David Ascher (:davida)
mozilla: review-
Details | Diff | Splinter Review
patch for review v6 (56.99 KB, patch)
2009-05-26 18:07 PDT, David Ascher (:davida)
mozilla: review-
Details | Diff | Splinter Review
patch for review v7 (56.76 KB, patch)
2009-05-27 14:58 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
[checked in] patch for review v8 (56.86 KB, patch)
2009-05-27 15:43 PDT, David Ascher (:davida)
mozilla: review+
Details | Diff | Splinter Review
mutiple messages mockup (99.72 KB, image/png)
2009-05-28 02:12 PDT, Andreas Nilsson (:andreasn)
no flags Details
updated mockup (100.11 KB, image/png)
2009-05-28 02:56 PDT, Andreas Nilsson (:andreasn)
no flags Details
fix for xul warning [checked in] (879 bytes, patch)
2009-05-28 10:58 PDT, David Ascher (:davida)
philringnalda: review+
Details | Diff | Splinter Review
mockup v4 (110.31 KB, image/png)
2009-05-29 07:30 PDT, Andreas Nilsson (:andreasn)
no flags Details
Buttons aren't with same height (21.16 KB, image/jpeg)
2009-05-29 08:49 PDT, [:Aureliano Buendía]
no flags Details
patch making things faster and prettier (30.00 KB, patch)
2009-05-31 00:00 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
_correct_ patch (30.58 KB, patch)
2009-05-31 00:01 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
Patch including connotent refactoring (33.06 KB, patch)
2009-06-01 10:48 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
mockup v5 (113.54 KB, image/png)
2009-06-01 15:23 PDT, Andreas Nilsson (:andreasn)
no flags Details
revised patch w/ thread circling and css splitting (36.14 KB, patch)
2009-06-02 15:23 PDT, David Ascher (:davida)
bugmail: review-
Details | Diff | Splinter Review
review comments fixed (38.55 KB, patch)
2009-06-02 18:00 PDT, David Ascher (:davida)
no flags Details | Diff | Splinter Review
works better with all the files (39.38 KB, patch)
2009-06-03 09:42 PDT, David Ascher (:davida)
bugmail: review+
mozilla: superreview+
Details | Diff | Splinter Review
[checked in] nits addressed (39.97 KB, patch)
2009-06-03 13:26 PDT, David Ascher (:davida)
davida: review+
davida: superreview+
Details | Diff | Splinter Review
workaround to deal w/ remote messages (esp. news) (8.24 KB, patch)
2009-06-04 12:13 PDT, David Ascher (:davida)
mozilla: review+
standard8: review-
Details | Diff | Splinter Review
some css fixes (2.67 KB, patch)
2009-06-08 11:08 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review

Description Bryan Clark (DevTools PM) [:clarkbw] 2008-09-11 10:42:37 PDT
When a collapsed thread is selected in the thread list the message pane displays the first message.  This behavior tends to make people think that despite their selecting of a collapsed thread, any actions performed will take place on the first message; because it is what is being shown.

With bug 448288 we'll be enabling actions on entire threads and yet we don't have a concept of how to visualize a thread.

Here's a plan for this. 
* When a collapsed thread is selected display a "Thread Summary" in the message pane
* When the thread is expanded individual messages should be displayed
* The "Thread Summary" should contain the thread of messages headers which are currently hidden in the collapsed thread list
* The "Thread Summary" should also display inline thread actions like ( Delete ) ( Tag ) and others.
* Each sender listed in the Thread Summary is linked to the message sent such that clicking on the reply will open the correct message and expand the thread in the thread list.

A basic layout for this Thread Summary could be something like this:

.----------------------------------------------.
|               ( Thread ) ( Actions ) ( Area) |
|                                              |
|  *Thread Subject*                            |
|    Sender Original        {date}             |
|      Sender Reply         {date}             |
|      Sender Reply         {date}             |
|        *Sender Reply*    *{date}*            |
|                                              |
'----------------------------------------------'

I think it would also make sense to look at including the text of the first message in the summary.  Perhaps making the first message text only show a preview of the first couple sentences.

More detailed mockups to come.
Comment 1 David :Bienvenu 2008-09-11 10:51:28 PDT
What do we show if the user selects multiple collapsed threads, or a mix of collapsed threads and messages in expanded threads? In the extreme of selecting all the messages, we might need to limit the number of summaries we show for performance reasons.
Comment 2 Bryan Clark (DevTools PM) [:clarkbw] 2008-09-11 12:00:54 PDT
(In reply to comment #1)
> What do we show if the user selects multiple collapsed threads, or a mix of
> collapsed threads and messages in expanded threads? In the extreme of selecting
> all the messages, we might need to limit the number of summaries we show for
> performance reasons.

I think that is a bit of a separate bug, but I would suggest we use a very similar method.  Currently if a user selects (for example) 3 threads and 2 messages our message pane view goes blank.

I would recommend that we build another "Selection Summary" page that displays the selected threads and messages with inline actions available. 

.----------------------------------------------.
|            ( Selection ) ( Actions ) ( Area) |
|            3 Threads, 2 Messages             |
|                                              |
|  Thread Subject           (10 messages)      |
|    Sender                 {date}             |
|                                              |
|  Thread Subject           (23 messages)      |
|    Sender                 {date}             |
|                                              |
|  Thread Subject           (16 messages)      |
|    Sender                 {date}             |
|                                              |
|  Message Subject                             |
|    Sender                 {date}             |
|                                              |
|  Message Subject                             |
|    Sender                 {date}             |
|                                              |
'----------------------------------------------'
Comment 3 Bryan Clark (DevTools PM) [:clarkbw] 2008-09-11 14:53:32 PDT
Also, this shares a lot of concepts with bug 452440
Comment 4 Ronald Killmer 2008-09-11 15:49:00 PDT
The layout in Comment #0 looks good as far as it goes. I think greater usefulness would be achieved if the Recipients replies stored in the Sent folder were interleaved, probably in a color. 

Make Read headers normal & default or filtered Tag color. The Unread headers bold & unread or filtered Tag color. Then the Recipient replies, italic and colored as Read or Tag color.

I suppose this would require Gloda to permit the inclusion of the Recipient replies.

As for the layout illustrated in Comment #2 I see that as being redundant of what is can be shown in the thread pane when using "Wide Message Pane" layout.  Perhaps with the vertical or the extension provided stacked layout the proposal offers a solution not now possible.

I do suspect there is a case where We will show a blank message pane. That case is the 'Start' page is disabled and *no* mail account set to Auto-Poll it's server.  I see this now in Shredder nightlies. Even the Account Manager page is not displayed until I do a mouse action in the window.
Comment 5 Magnus Melin 2008-09-12 08:24:57 PDT
xref bug 364896 for the outlook/the bat multi selection behavior, which arguably has it's sides.
Comment 6 Magnus Melin 2008-09-12 08:27:34 PDT
... though that's for multiple single message selection, not threads.
Comment 7 Onno Ekker [:nONoNonO UTC+1] 2008-09-12 08:46:19 PDT
This summary would also fit very nice on mail digests, especially if the individual messages are clickable. See also bug 297088 and bug 147567.
Comment 8 David :Bienvenu 2008-09-12 13:59:21 PDT
Do we want to detect the case where the user has a collapsed thread selected and then expands the thread, and load the top level message in the thread, so the user can read it? 

>* The "Thread Summary" should contain the thread of messages headers which are
>currently hidden in the collapsed thread list

Shouldn't it also contain a line for the top level message in the thread, so the user can click on it to read it, or otherwise manipulate it?
Comment 9 Bryan Clark (DevTools PM) [:clarkbw] 2008-09-12 15:36:35 PDT
> Do we want to detect the case where the user has a collapsed thread selected
> and then expands the thread, and load the top level message in the thread, so
> the user can read it? 

> Shouldn't it also contain a line for the top level message in the thread, so
> the user can click on it to read it, or otherwise manipulate it?

Right and right.
Comment 10 Mike Cowperthwaite 2008-09-14 12:59:42 PDT
(In reply to comment #0)
> When a collapsed thread is selected in the thread list the message pane
> displays the first message.

As far as I know, a collapsed thread is not selected when you click the top message; only the first message is selected.  If you click the thread icon on a collapsed thread, the entire thread is selected and the thread is expanded.  I read bug 448288 comment 4 and I couldn't tell from your "right now" whether you meant in the current builds or with the patch from that bug applied; it seemed like the former.


(In reply to comment #8)
> Do we want to detect the case where the user has a collapsed thread selected
> and then expands the thread, and load the top level message in the thread, so
> the user can read it? 

If the top level message is going to be loaded, then the selection *must* be changed to only include the top-level message.  There is already too much disconnect between what's selected and what's shown in the message pane.  This bug's basic idea of displaying something in the pane when a thread is selected works towards solving some of that disconnect.  A general-purpose multiple-selection display (bug 452440) would be even better.

Introducing more disconnect is counterproductive.  A message displayed in the message pane *must* mean that a single message is selected in the thread pane.
There are several old bugs complaining about a message continuing to be displayed when the selection has been cleared or for other reasons
Comment 11 Mike Cowperthwaite 2008-09-14 13:06:34 PDT
(In reply to comment #10)
> A general-purpose
> multiple-selection display (bug 452440) would be even better.

Er, bug 451251.
Comment 12 Wayne Mery (:wsmwk, NI for questions) 2009-01-11 17:43:50 PST
Brian, in bug 214111 Comment 18 dmose writes "we probably need to make all such  [collapsed thread] changes in the same release or risk serious confusion." 

Bug 214111 is marked wanted-thunderbird3+, target rc1. But this bug, and it's dependent bug 448288 (which has a patch in the making), are not.  You might want them all to have the same target.  (there may be other such bugs - I didn't search)
Comment 13 David Ascher (:davida) 2009-01-21 17:58:27 PST
taking because i seem to care about this issue.  we'll see where that takes us.
Comment 14 David Ascher (:davida) 2009-01-27 00:49:03 PST
Created attachment 359010 [details] [diff] [review]
WIP

checkpointing WIP.  needs attachment 359009 [details] [diff] [review] from bug 448288.

just does summary view, no buttons yet, and lots of rough edges.
Comment 15 David Ascher (:davida) 2009-01-28 00:11:48 PST
Created attachment 359227 [details] [diff] [review]
WIP v2

Updated version.  does delete, archive, junk. 

I spent a long time ripping out the iframe implementation to make it easier for add-on authors to extend and to simplify the code, but the layout implications didn't make it work -- if one selected "all", then the flex rules forced the message pane to take over, wouldn't scroll, etc.  xul, xul. sigh.

anyway, html works just fine.
Comment 16 David Ascher (:davida) 2009-01-28 00:12:51 PST
Created attachment 359229 [details]
screenshot

screenshot of WIP v2.  bunch of details to tweak still.
Comment 17 David Ascher (:davida) 2009-01-28 07:53:52 PST
I'd like to get this in b2, but i'm going to set milestone at b3 in case wacky things show up.
Comment 18 David Ascher (:davida) 2009-02-23 13:40:53 PST
Created attachment 363756 [details] [diff] [review]
jquery patch
Comment 19 David Ascher (:davida) 2009-02-23 13:44:06 PST
Created attachment 363757 [details]
WIP v3

Updated patch.  This requires a bunch of related patches, including those in bugs 448288 & 476103, and the jquery patch attached here.

It's far from finished, but I'm finding it it useful nonetheless.
Comment 20 David Ascher (:davida) 2009-02-23 14:06:37 PST
Created attachment 363762 [details] [diff] [review]
WIP v4

It'd be better if I didn't forget files.
Comment 21 David Ascher (:davida) 2009-02-23 14:14:33 PST
Created attachment 363765 [details] [diff] [review]
WIP v5

redux
Comment 22 David Ascher (:davida) 2009-02-23 16:05:03 PST
Created attachment 363785 [details] [diff] [review]
WIP v6

this version deals w/ correctly displaying tags as they get changed w/ keyboard shortcuts for example, modulo some gloda bugs.

it doesn't deal w/ detecting read/unread state changes due to bug 479890.
Comment 23 David Ascher (:davida) 2009-05-01 16:53:43 PDT
Created attachment 375428 [details] [diff] [review]
WIP v7

* handle starring, tags of all messages in a thread in the multimessage view.

* display number of messages in the thread view, as per a request from bienvenu
Comment 24 Magnus Melin 2009-05-02 09:40:44 PDT
Not that jquery isn't a nice framework, but that doesn't mean we should ship with it...
Comment 25 Andrew Sutherland [:asuth] 2009-05-03 14:51:03 PDT
(In reply to comment #24)
> Not that jquery isn't a nice framework, but that doesn't mean we should ship
> with it...

What downsides are there to shipping with it?  Somes updsides are that it enables rapid development and lots of people are familiar with it, which seem like two pretty big upsides.
Comment 26 David Ascher (:davida) 2009-05-04 09:46:42 PDT
Created attachment 375625 [details] [diff] [review]
WIP v8

Version that doesn't require jquery, and w/ a bunch more fixes.

Still left to do: finish l10n, and do non-mac themes.

(I think that jquery in the tb platform makes a great deal of sense, and should be discussed, but this isn't the bug to do it in, or the forum).
Comment 27 Magnus Melin 2009-05-04 10:08:46 PDT
I'm not saying we necessarily shouldn't, but it's a decent amount of code we'd essentially have to maintain.
Frameworks are great esp. for inter-browser interoperability but if something in the framework is really essential - shouldn't such a feature belong in core (without any framework) and preferably be standardized?
Comment 28 David Ascher (:davida) 2009-05-04 17:03:53 PDT
Created attachment 375733 [details] [diff] [review]
WIP v9

Two things left to do:

1) if selecting a collapsed thread, then clicking on the name of the first poster, the thread uncollapses (good), the first message is selected (good), but the message pane still shows the group.  I suspect that's because SelectionChanged() isn't called in nsMsgDBView.cpp, but I don't know why.

2) as discussed on IRC, "S" for starring/unstarring only affects the first message in a collapsed thread, not all.

bienvenu, i'd love your suggestions for both of these issues.

Apart from that, AFAICT, it's review-ready.
Comment 29 David Ascher (:davida) 2009-05-04 17:06:36 PDT
Created attachment 375734 [details]
updated screenshot
Comment 30 David :Bienvenu 2009-05-04 17:08:32 PDT
2) is just a bug with my multi-selection patch - I need to make star turn the selection into msg hdrs like we do for the other commands.

1) I'll have to look into, which I'll do soon, after I finish up some work with gmail integration.
Comment 31 David Ascher (:davida) 2009-05-04 18:40:55 PDT
A couple of additional notes from my bike ride home:

1) I should probably cap the number of messages/threads to display, for those cases when people select 10,000 rows, with that cap picked from a pref or somewhere.

2) I think it'd be good to provide hooks for extension authors to display different content in that frame, both for the multi-select, as well as the folder-select case.  I might have a poke at that, time permitting.

Neither of those should block b3, IMO.
Comment 32 Andrew Sutherland [:asuth] 2009-05-05 07:39:27 PDT
I think summarizeSelection should live on nsIMsgDBViewCommandUpdater, not nsIMsgWindowCommands.

Use of gSummary is sketchy but arguably it's no more work to change it from that to the 'right way' when I (or whomever) refactors the thing than if you stored things per-tab.

You create a few JS classes that start with lowercase letters; they should probably have initial caps.  Along the same lines, you add some global functions that start with a lowercase char and some that start with an uppercase character.  Might as well be consistent, whatever you do there.  (I suppose we have a lot of initial-caps ones right now, but I claim they are wrong.  I also am deleting many of them in a refactoring, so arguably they won't be there long.)

More doxygen docs would be appreciated, if only to explain what functionality justifies the existence of the classes/functions.  One of the deficiencies of the current front-end code is that it's not clear why various pieces of code exist, even if the function name and arguments are reasonable.
Comment 33 David :Bienvenu 2009-05-05 18:00:17 PDT
(In reply to comment #28)
> 
> 1) if selecting a collapsed thread, then clicking on the name of the first
> poster, the thread uncollapses (good), the first message is selected (good),
> but the message pane still shows the group.  I suspect that's because
> SelectionChanged() isn't called in nsMsgDBView.cpp, but I don't know why.

SelectionChanged is called from the js code. I see you're calling SelectFolderMsgByKey, which is poking the tree selection, but technically the selection hasn't changed, has it? By first message, do you mean the very first message in the thread? I suppose we need to create some artificial selection changed notification in this case, though it's not clear to me where these smarts should live - does nsMsgDBView really know that something happened with summarizeSelection? (and I'm guessing that SeaMonkey is going to need change its nsIMsgDBViewCommandUpdater, or nsIMsgWindowCommands to add a summarizeSelection method - or does xpconnect simply ignore methods that don't exist when calling into js?)

> 
> 2) as discussed on IRC, "S" for starring/unstarring only affects the first
> message in a collapsed thread, not all.
>
I have a fix for this, but the general fix for other commands is a bit trickier - mark read/unread, thread as read, junk/non junk (especially tricky).
Comment 34 David Ascher (:davida) 2009-05-05 18:05:40 PDT
Created attachment 375918 [details] [diff] [review]
WIP v10

asuth's review comments included.

Note: I include the minimal changes to /suite to make them have API endpoints for the new nsIMsgDBViewCommandUpdater interface, but there is no summarizing in the UI.  I'm hoping that's the right thing to do.
Comment 35 David :Bienvenu 2009-05-07 16:40:20 PDT
I'm not seeing a trash icon, and the archive button doesn't look like a button - looks like the theming isn't working on windows, I guess. I'll look into that. I'm also not seeing Stars in the message pane - I hope that's the theming as well. Are you getting the star from the msg hdr flags, or gloda?

Threads with '&' in the subject don't display because '&' isn't getting escaped. There might be a few other chars that need to be escaped as well.
Comment 36 David :Bienvenu 2009-05-07 17:20:09 PDT
qute doesn't have folder-trash.png

some of the new copyrights refer to 2008

I've fixed a few js warnings locally, and cleaned up some other things. I'll try to figure out how to get just those diffs to you...
Comment 37 David :Bienvenu 2009-05-08 11:41:56 PDT
I see this assertion relatively often, and if assertions pop up a dialog, I'm effectively horked and I have to restart:

 	xpcom_core.dll!NS_DebugBreak_P(unsigned int aSeverity=0x00000001, const char * aStr=0x5f43a490, const char * aExpr=0x5f43a488, const char * aFile=0x5f43a438, int aLine=0x0000052c)  Line 364 + 0xc bytes	C++
 	gklayout.dll!nsEventStateManager::PreHandleEvent(nsPresContext * aPresContext=0x03eb0e00, nsEvent * aEvent=0x0021dd18, nsIFrame * aTargetFrame=0x03e9802c, nsEventStatus * aStatus=0x0021db14, nsIView * aView=0x03ec9e88)  Line 1324 + 0x2b bytes	C++
 	gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0021dd18, nsIView * aView=0x03ec9e88, nsEventStatus * aStatus=0x0021db14)  Line 6126 + 0x36 bytes	C++
 	gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x03ec9e88, nsGUIEvent * aEvent=0x0021dd18, nsEventStatus * aEventStatus=0x0021db14)  Line 5936 + 0x1a bytes	C++
 	gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x03ec9e88, nsPoint aPoint={...}, nsGUIEvent * aEvent=0x0021dd18, int aCaptured=0x00000000)  Line 1397	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0021dd18, nsEventStatus * aStatus=0x0021dc5c)  Line 1353 + 0x22 bytes	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0021dd18)  Line 170	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0021dd18, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1052 + 0xc bytes	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0021dd18)  Line 1073	C++
 	gkwidget.dll!nsWindow::DispatchFocus(unsigned int aEventType=0x0000006b, int isMozWindowTakingFocus=0x00000001)  Line 6677 + 0x11 bytes	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=0x00000007, unsigned int wParam=0x00440736, long lParam=0x00000000, long * aRetValue=0x0021e228)  Line 4854 + 0x17 bytes	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x00360d14, unsigned int msg=0x00000007, unsigned int wParam=0x00440736, long lParam=0x00000000)  Line 1268 + 0x1d bytes	C++
Comment 38 David :Bienvenu 2009-05-08 12:23:01 PDT
(In reply to comment #28)

> 1) if selecting a collapsed thread, then clicking on the name of the first
> poster, the thread uncollapses (good), the first message is selected (good),
> but the message pane still shows the group.  I suspect that's because
> SelectionChanged() isn't called in nsMsgDBView.cpp, but I don't know why.
> 
I have a fix for this, though it's all in selectionSummaries.js.  Calling selectionChanged from the backend produced the assertions above so I had to do it all in the front end...
Comment 39 David :Bienvenu 2009-05-11 12:53:59 PDT
Created attachment 376768 [details] [diff] [review]
this includes my patch for handling selection, and tweaks davida's patch a bit, including fixing the selection issue

I don't know how happy interdiff will be with this, since it includes an other patch.
Comment 40 David :Bienvenu 2009-05-11 13:49:44 PDT
ok, interdiff very unhappy - I'll try to generate a patch with the same files as are in wip 10
Comment 41 David :Bienvenu 2009-05-11 14:12:16 PDT
Created attachment 376779 [details] [diff] [review]
diff for interdiff
Comment 42 David :Bienvenu 2009-05-11 14:14:32 PDT
ok, interdiff between that last patch and wip10 shows my changes - mostly silencing warnings by declaring variables, and the fix to make the summary page work with selection.
Comment 43 David Ascher (:davida) 2009-05-13 12:16:03 PDT
Created attachment 377225 [details] [diff] [review]
WIP v11

Ok, this version includes bienvenu's fixes, as well as:
 - use the mime2Decoded accessors for subjects & authors
 - works in qute, gnomestripe, and pinstripe

I'm thinking about a possible tweak to the HTML to make it easier for addons to place visualizations and other things like this, as well as something like the 'analyzer' model that I have in the patch for bug 492158, but I'd like bienvenu's thoughts on what else is needed at this point.

(also: andreas, this patch would be a good place for you to work on making it it prettier!)
Comment 44 David Ascher (:davida) 2009-05-13 13:30:10 PDT
Created attachment 377240 [details] [diff] [review]
WIP v12

Use the right patch, luke.
Comment 45 Bryan Clark (DevTools PM) [:clarkbw] 2009-05-14 11:13:33 PDT
andreas: can you hg qimport this patch to get an idea for what it does?  You can either edit the page directly or do the styling outside this patch and we can pull it in.  Thanks!
Comment 46 Bryan Clark (DevTools PM) [:clarkbw] 2009-05-19 13:02:35 PDT
assigning to andreas for now.  once we have a cleaned up look we can throw this back to david to finish up
Comment 47 David :Bienvenu 2009-05-20 18:59:00 PDT
I've landed the grouping actions fix - let me know if there's stuff you need from me for this bug.
Comment 48 David Ascher (:davida) 2009-05-21 12:13:10 PDT
Created attachment 378926 [details] [diff] [review]
patch for review v1

At this point, I think the patch is ready for your review.  I've taken care of asuth's drive-by comments in comment #32.

I suspect andreas and bryan will want to tweak the layout & css, but the code review is orthogonal.
Comment 49 David :Bienvenu 2009-05-21 13:07:31 PDT
Error ``MsgHdrToMimeMessage is not defined'' [x-] in file ``chrome://messenger/content/selectionsummaries.js'', line 394, character 0.

might explain why I'm not seeing snippets
Comment 50 David :Bienvenu 2009-05-21 13:38:32 PDT
I'm also not seeing dumpExc, which looks to be defined in GlodaTestHelper.js but not anywhere relevant to this patch...
Comment 51 David Ascher (:davida) 2009-05-21 13:55:08 PDT
Created attachment 378944 [details] [diff] [review]
patch for review v2

fix subjects containing &'s
fix loading of mimemsg.js
Comment 52 David :Bienvenu 2009-05-21 14:00:13 PDT
Comment on attachment 378944 [details] [diff] [review]
patch for review v2

while I'm waiting for my rebuild, Phil would want me say that we include trailing ';' on our statements; You're missing those in  a few places...
Comment 53 Phil Ringnalda (:philor) 2009-05-21 14:46:36 PDT
http://beaufour.dk/jst-review/?patch=378944 has some advice, too, only some of which is bogus.
Comment 54 David :Bienvenu 2009-05-21 16:33:37 PDT
yes, the W's and X ones at least...
Comment 55 David Ascher (:davida) 2009-05-21 16:56:41 PDT
Created attachment 378999 [details] [diff] [review]
patch for review v3

fix review nits from jst-in-the-cloud
make the behavior depend on the pref as discussed in IRC
fix wrapping oddity when dealing with long subjects
fix unstarring
Comment 56 David :Bienvenu 2009-05-21 17:52:07 PDT
if I turn the pref off, multi-selection doesn't clear the message pane anymore

missing ; at eol on these lines:
+  return document.getElementById(visiblePaneId)

+    for (var i = 0; i < this._msgURIs.length; ++i)
+      this._msgHdrs.push(messenger.msgHdrFromURI(this._msgURIs[i]))

+    if ((senderName[0] == "'" && senderName[senderName.length-1] == "'") ||
+        (senderName[0] == '"' && senderName[senderName.length-1] == '"'))
+      senderName = senderName.slice(1, -1)

+      if (countUnread)
+        _mm_addClass(msg, 'unread')

+      subject.insertBefore(star, subject.firstChild);
+      wrappedsubject.appendChild(subject)

+          label += PluralForm.get(numMsgs, getStr("countUnread")).replace('#1', countUnread);
+        label += ")"

+      if (this._snippetNodes) {
+        let snippetNode = this._snippetNodes[domkey]

+        // for tags, we need to do some fancy stuff, to figure out the set
+        // of tags that correspond to all of the messages in a collapsed
+        // thread.
+        let key = messageKey + glodaMsg.folder.uri

+      msg = htmlpane.contentDocument.createElement("div");
+      let key = msgHdr.messageKey + msgHdr.folder.URI

+      wrappedsender.appendChild(star);
+      wrappedsender.appendChild(sender)

+      if (msgHdr.threadId != firstThreadId) // at least more than one thread
+        return summarizeMultipleSelection(selectedMsgUris)

no need for braces here:
+            if (aMimeMsg == null) {
+              return;
+            }

or here:
+    while (messagesElt.firstChild) {
+      messagesElt.removeChild(messagesElt.firstChild);
+    }
+
or here:
+    if (! gPrefBranch.getBoolPref("mail.operate_on_msgs_in_collapsed_threads")) {
+      return;
+    }

or here:
+  if (mCommandUpdater) {
+    mCommandUpdater->SummarizeSelection();
+  }


need newline at eof here:

+countUnread=, #1 unread;, #1 unread
+Nmessages=(#1 message);(#1 messages)
\ No newline at end of file

the long subject wrapping looks like this:

   Foo
   Bar long line...
  continue long subject

I would expect the second line to line up with the first line, but if the wrapping was intentional, I'll defer to you and Bryan.
Comment 57 David Ascher (:davida) 2009-05-21 18:18:23 PDT
Created attachment 379013 [details] [diff] [review]
patch for review v4

Thanks.  Apologies for the missing trailing semicolons, that's years of Python training biting me.  Nits all fixed, I think.

Pref behavior fixed.  Hanging indent problem fixed through the judicious use of negative text-indent.  I've now reached CSS brown-belt.
Comment 58 David :Bienvenu 2009-05-22 07:53:18 PDT
years of c++ programming have made the missing trailing semicolons easy to spot :-)

This patch fixes the issues I was seeing. I thought I was seeing a lot less snippets than I was before, but I think that older messages just don't have snippets, even though they have gloda id's. I don't think I ran with Andrew's new gloda stuff that invalidates gloda db's on this machine, but I could be wrong.

Ugh, subjects with '&' in them get displayed in the summary as &amp; Much better than throwing an exception, but not ideal :-) < and > show up as &lt; etc.


<cntrl a> is really slow, and I see a ton of warnings on the console: WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLa
youtPhase_FrameC] == 0', file c:\mozilla-build\msys\tbirdhq\mozilla\layout\base\
nsPresContext.h, line 1026

I see this warning whenever I add a message to the selection, but cntrl a adds a lot of messages so I guess it slows down.

Why are we revving the uuid here?

-[scriptable, uuid(7B8F4A65-CFC4-4b3f-BF5C-152AA8D5CD10)]
+[scriptable, uuid(6844818d-297e-40e3-bcaa-273f4aa22d8c)]
 interface nsIMsgWindowCommands : nsISupports {
   void selectFolder(in ACString folderUri);
   void selectMessage(in ACString messageUri);
   void clearMsgPane();
 };

I can't believe I'm saying this, but you should K&R the brace after the else :-)

+      if (! threads[msgHdr.threadId]) {
+        threads[msgHdr.threadId] = [msgHdr];
+        numThreads += 1;
+      }
+      else
+      {
+        threads[msgHdr.threadId].push(msgHdr);
+      }

These are all nits, but I would like to fix the entity stuff before landing.
Comment 59 David :Bienvenu 2009-05-22 11:50:48 PDT
one other small glitch. I did a select all at one point, and the message summary accurately pointed out:

(Note: 2614 messages are selected, the first 100 are shown)

But now it won't stop telling me that, even though I only have two messages selected :-)
Comment 60 David Ascher (:davida) 2009-05-22 14:02:31 PDT
Created attachment 379238 [details] [diff] [review]
patch for review v5

all known issues dealt with (in bug + irc + clarkbw comments), including the fact that we weren't listing authors in the multi-message case, oops.  Stars have moved to the right, where they cause a lot less trouble.
Comment 61 David :Bienvenu 2009-05-22 14:42:36 PDT
I'm still seeing -  (Note: 1478 messages are selected, the first 100 are shown) even if I just have a couple messages selected. I need to restart to get this to go away, after doing a select all.
Comment 62 David :Bienvenu 2009-05-26 14:21:53 PDT
Comment on attachment 379238 [details] [diff] [review]
patch for review v5

needs new patch for the counts issue, and some theming issues davida encountered. We're really close, though - it's a race w/ gloda!
Comment 63 David Ascher (:davida) 2009-05-26 18:07:20 PDT
Created attachment 379795 [details] [diff] [review]
patch for review v6

Ok, so getting the theme include stuff to work right took a long time, but the css looks better now.  The only thing that bugs me a bit about this layout (done w/ clarkbw) is that if you select starred messages & some threads, the stars aren't where I think they should be.  But I think that could land in a subsequent patch, as I expect we'll want to iterate on the css anyway.
Comment 64 David :Bienvenu 2009-05-27 07:42:58 PDT
The archive button on Vista doesn't look good w/ this patch:

http://www.grabup.com/uploads/33002722051920092A0769429C22.png
Comment 65 David :Bienvenu 2009-05-27 07:54:26 PDT
Comment on attachment 379795 [details] [diff] [review]
patch for review v6

This shouldn't be part of this patch, right?

@@ -315,6 +318,7 @@
       let progressCount = 0;
 
       // For each activity that is in progress, get its status.
+      /*
       this._activeProcesses.forEach(function (element) {
           if (element.state ==
               Components.interfaces.nsIActivityProcess.STATE_INPROGRESS &&
@@ -323,7 +327,7 @@
             ++progressCount;
           }
         });
-
+     */

really long subjects don't play well with the buttons - http://www.grabup.com/uploads/98002722051920092A0769539C07.png

Once we deal with the aforemention issues, I think we're good to go.
Comment 66 David :Bienvenu 2009-05-27 07:57:52 PDT
When we're in multi-selection mode, and we have subject and sender for each message, what should clicking on the linkified sender do? It doesn't seem to do anything for me.
Comment 67 David :Bienvenu 2009-05-27 08:01:14 PDT
Sorry to keep spewing issues one by one - if I have three contiguous messages selected, and I click on the second or third subject link to load that message, we first display the first message in the selection, and then the message corresponding to the clicked on link.
Comment 68 David :Bienvenu 2009-05-27 08:30:21 PDT
(In reply to comment #67)
> Sorry to keep spewing issues one by one - if I have three contiguous messages
> selected, and I click on the second or third subject link to load that message,
> we first display the first message in the selection, and then the message
> corresponding to the clicked on link.

This is a result of the way we switch between the iframe and the vbox - we switch to the vbox, which displays the previously displayed message, and then we select and display the newly selected message. Would it be possible to switch after we've loaded the new message, or at least after we've kicked off the load of the new message, to reduce the flicker?
Comment 69 David Ascher (:davida) 2009-05-27 10:04:33 PDT
re: comment #65: right, sorry -- that code was triggering an unrelated JS assertion on linux, so i commented it out to get this patch in shape, but it's unrelated.; will fix wrapping of long subjects


re: comments #67,68: Hmm, all I'm doing there is selectFolderMsgByKey() on the dbview, and the iframe gets swapped out as a result of the call to SelectionChanged() in nsMsgDBView.cpp.  

Could you try something like:

--- a/mail/base/content/msgMail3PaneWindow.js
+++ b/mail/base/content/msgMail3PaneWindow.js
@@ -501,21 +501,21 @@ function LoadCurrentlyDisplayedMessage()
 function LoadCurrentlyDisplayedMessage()
 {
   var scrolled = (gCurrentlyDisplayedMessage != nsMsgViewIndex_None);
   if (scrolled)
   {
     var treeView = gDBView.QueryInterface(Components.interfaces.nsITreeView);
     var treeSelection = treeView.selection;
     treeSelection.select(gCurrentlyDisplayedMessage);
-    if (treeView)
-      treeView.selectionChanged();
     EnsureRowInThreadTreeIsVisible(gCurrentlyDisplayedMessage);
     SetFocusThreadPane();
     gCurrentlyDisplayedMessage = nsMsgViewIndex_None; //reset
+    if (treeView)
+      treeView.selectionChanged();
   }
   return scrolled;
 }
 
 function IsCurrentLoadedFolder(folder)
 {
   var msgfolder = folder.QueryInterface(Components.interfaces.nsIMsgFolder);
   var currentLoadedFolder = GetThreadPaneFolder();


I'm not seeing the flicker myself, probably cause i'm not running a debug build.
Comment 70 David :Bienvenu 2009-05-27 12:16:50 PDT
that tweak didn't help - I'll poke around a little.
Comment 71 David :Bienvenu 2009-05-27 12:35:55 PDT
even w/o your tweak, we're starting the load of the new message before calling summarizeSelection; I guess the issue is that loading is async, and switching to the message pane is sync. We'll just have to see if it's an issue for anyone with release builds but I certainly wouldn't hold up this patch for it.
Comment 72 David Ascher (:davida) 2009-05-27 14:58:45 PDT
Created attachment 379971 [details] [diff] [review]
patch for review v7

Ok, so this should address the subject wrapping, the archive/trash icon button size issues (at least on XP, I don't have vista), and the extraneous commenting out.
Comment 73 David Ascher (:davida) 2009-05-27 15:43:17 PDT
Created attachment 379990 [details] [diff] [review]
[checked in] patch for review v8

somehow i forgot the pinstripe changes to make the trash button size properly.
Comment 74 David :Bienvenu 2009-05-27 16:27:53 PDT
Comment on attachment 379990 [details] [diff] [review]
[checked in] patch for review v8

looks good to me...
Comment 75 David Ascher (:davida) 2009-05-27 19:36:32 PDT
So, before we check this in, I'd like to do a bit more testing on linux -- my last experiment showed some issues w/ the stars, but my linux VM was very unstable.  I may rope asuth or clarkbw in testing the patch if I can't fix that.
Comment 76 David Ascher (:davida) 2009-05-27 19:51:16 PDT
Test plan notes:

1) select multiple messages

--> should see the message pane display a summary of these messages (subject, author, star if starred, tags if tagged, and first few words of the body).

The summary also shows the total size of messages (news messages don't advertise their size, so expect those number to be off for usenet messages)

2) change selection

--> summary should update and reflect the new selection.

3) select a lot of messages

--> summary will show only first 100, and have a note about that at the bottom

4) select fewer messages

--> summary won't show that note at the bottom

5) select multiple message including several in the same thread

--> summary will represent the messages in the thread as one line, indicating how many messages in that thread are selected (not the number of messages in the thread total)

6) select one collapsed thread

--> summary will summarize the thread, showing initial thread title at the top, author of each message, and date of each message.

7) in thread summary, clicking on a message should select it (and display it)

8) in multiple-message summary including a thread, clicking on the thread name will select the entire thread (leaving it collapsed if collapsed, or leaving it uncollaped if not), and display the thread summary

9) in multiple-messages summary including _some_ messages from a thread, clicking on the thread description will select the subset of messages from that thread that are in the current selection, and display the thread summary.

10) if the "mail.operate_on_msgs_in_collapsed_threads" pref is set to false, then the thread summary won't show for collapsed threads.  Note: multiple message summaries won't be shown either if this pref is false.


11) in the multi-message-with-threads condition, thread summaries show stars if any of the selected messages in that thread have stars.  They also show the union of all tags assigned to those messages.

12) in both multi-message and thread summary views, hitting "S" to toggle starred/unstarred, hitting the number keys to set tags, will dynamically update the summary, as long as the full-text indexer (gloda) is enabled.

13) the snippets (first line of body) require gloda to be enabled.
Comment 77 David Ascher (:davida) 2009-05-27 22:27:44 PDT
With a clean build linux checks out just fine.
Comment 78 Mark Banner (:standard8, afk until Dec) 2009-05-28 00:43:21 PDT
I'm not convinced this will work with rtl locales:

+      if (numMsgs > 1) {
+        let label = "(";
+        label += PluralForm.get(numMsgs, gSelectionSummaryStrings["numMsgs"]).replace('#1', numMsgs);
+        if (countUnread)
+          label += PluralForm.get(numMsgs, gSelectionSummaryStrings["countUnread"]).replace('#1', countUnread);
+        label += ")";
+        countNode.innerHTML = label;
+      }

However, I think that could be spun off into a follow up patch if it is a problem as I don't think it'll affect those strings already defined.
Comment 79 Mark Banner (:standard8, afk until Dec) 2009-05-28 01:43:50 PDT
Comment on attachment 379990 [details] [diff] [review]
[checked in] patch for review v8

Checked in: http://hg.mozilla.org/comm-central/rev/3c87898162a3

Note that I noticed the line endings in mail/themes/qute/jar.mn were messed up, so I fixed those.
Comment 80 Mark Banner (:standard8, afk until Dec) 2009-05-28 01:48:10 PDT
Leaving open for now - does Andreas still need to work on the visual design?
Comment 81 Andreas Nilsson (:andreasn) 2009-05-28 01:50:46 PDT
Mark: Working on it right now. Will attach shortly.
Comment 82 Andreas Nilsson (:andreasn) 2009-05-28 02:12:10 PDT
Created attachment 380077 [details]
mutiple messages mockup

Visual design mockup.
* Tags appear next to message, hopefully saves a bit vertical space.
* Gray rectangle to indicate thread.
Comment 83 Mark Banner (:standard8, afk until Dec) 2009-05-28 02:24:07 PDT
(In reply to comment #82)
> Created an attachment (id=380077) [details]
> mutiple messages mockup

I realise this is only a mockup, but to me that 'X' button would be confusing - we normally use 'X' to signify closing something, not deleting.
Comment 84 Andreas Nilsson (:andreasn) 2009-05-28 02:56:18 PDT
Created attachment 380086 [details]
updated mockup

Mark: yes, I added it as a placeholder and missed to fix it before I attached it to the big. The button should use the theme's delete icon, just as it does now.
Comment 85 Ludovic Hirlimann [:Usul] 2009-05-28 06:22:47 PDT
(In reply to comment #76)
> Test plan notes:
> 
> 1) select multiple messages
> 
> --> should see the message pane display a summary of these messages (subject,
> author, star if starred, tags if tagged, and first few words of the body).
> 
> The summary also shows the total size of messages (news messages don't
> advertise their size, so expect those number to be off for usenet messages)

https://litmus.mozilla.org/show_test.cgi?id=7743
 
> 2) change selection
> 
> --> summary should update and reflect the new selection.

https://litmus.mozilla.org/show_test.cgi?id=7744
 
> 3) select a lot of messages
> 
> --> summary will show only first 100, and have a note about that at the bottom
> 
https://litmus.mozilla.org/show_test.cgi?id=7745
> 4) select fewer messages
> 
> --> summary won't show that note at the bottom
> 

https://litmus.mozilla.org/show_test.cgi?id=7746

> 5) select multiple message including several in the same thread
> 
> --> summary will represent the messages in the thread as one line, indicating
> how many messages in that thread are selected (not the number of messages in
> the thread total)

https://litmus.mozilla.org/show_test.cgi?id=7748


> 6) select one collapsed thread
> 
> --> summary will summarize the thread, showing initial thread title at the top,
> author of each message, and date of each message.

https://litmus.mozilla.org/show_test.cgi?id=7747

> 
> 7) in thread summary, clicking on a message should select it (and display it)

https://litmus.mozilla.org/show_test.cgi?id=7749
 
> 8) in multiple-message summary including a thread, clicking on the thread name
> will select the entire thread (leaving it collapsed if collapsed, or leaving it
> uncollaped if not), and display the thread summary


https://litmus.mozilla.org/show_test.cgi?id=7750

> 9) in multiple-messages summary including _some_ messages from a thread,
> clicking on the thread description will select the subset of messages from that
> thread that are in the current selection, and display the thread summary.


https://litmus.mozilla.org/show_test.cgi?id=7752

> 10) if the "mail.operate_on_msgs_in_collapsed_threads" pref is set to false,
> then the thread summary won't show for collapsed threads.  Note: multiple
> message summaries won't be shown either if this pref is false.

So we will get what happens in TB2 3.0b3 , right ?

https://litmus.mozilla.org/show_test.cgi?id=7751

> 11) in the multi-message-with-threads condition, thread summaries show stars if
> any of the selected messages in that thread have stars.  They also show the
> union of all tags assigned to those messages.

so for a thread , it is stared if any messages is stared and the tags shown is the sum of tags ?

> 
> 12) in both multi-message and thread summary views, hitting "S" to toggle
> starred/unstarred, hitting the number keys to set tags, will dynamically update
> the summary, as long as the full-text indexer (gloda) is enabled.

What happens if gloda is off ?

> 13) the snippets (first line of body) require gloda to be enabled.

dito ?
Comment 86 David Ascher (:davida) 2009-05-28 07:44:36 PDT
(In reply to comment #85)

> > 10) if the "mail.operate_on_msgs_in_collapsed_threads" pref is set to false,
> > then the thread summary won't show for collapsed threads.  Note: multiple
> > message summaries won't be shown either if this pref is false.
> 
> So we will get what happens in TB2 3.0b3 , right ?

Right, and TB2.

> 
> https://litmus.mozilla.org/show_test.cgi?id=7751
> 
> > 11) in the multi-message-with-threads condition, thread summaries show stars if
> > any of the selected messages in that thread have stars.  They also show the
> > union of all tags assigned to those messages.
> 
> so for a thread , it is stared if any messages is stared and the tags shown is
> the sum of tags ?

union - if two messages are starred "important", only one "important shows up.

> 
> > 
> > 12) in both multi-message and thread summary views, hitting "S" to toggle
> > starred/unstarred, hitting the number keys to set tags, will dynamically update
> > the summary, as long as the full-text indexer (gloda) is enabled.
> 
> What happens if gloda is off ?

The messages will be starred, but the display won't be up to date.

> > 13) the snippets (first line of body) require gloda to be enabled.
> 
> dito ?

W/o gloda, no snippets.
Comment 87 David Ascher (:davida) 2009-05-28 07:46:48 PDT
(In reply to comment #84)
> Created an attachment (id=380086) [details]
> updated mockup
> 

So, in earlier code, the stars were on the left as you have them, but it proved really hard for me anyway to do the wrapping right.  Now that I've rediscovered table layouts, though I think we can probably get it to work.  ;-)
Comment 88 Ludovic Hirlimann [:Usul] 2009-05-28 09:41:56 PDT
(In reply to comment #86)
> > > 11) in the multi-message-with-threads condition, thread summaries show stars if
> > > any of the selected messages in that thread have stars.  They also show the
> > > union of all tags assigned to those messages.
> > 
> > so for a thread , it is stared if any messages is stared and the tags shown is
> > the sum of tags ?
> 
> union - if two messages are starred "important", only one "important shows up.

https://litmus.mozilla.org/show_test.cgi?id=7753


> > > 12) in both multi-message and thread summary views, hitting "S" to toggle
> > > starred/unstarred, hitting the number keys to set tags, will dynamically update
> > > the summary, as long as the full-text indexer (gloda) is enabled.
> > 
> > What happens if gloda is off ?
> 
> The messages will be starred, but the display won't be up to date.

https://litmus.mozilla.org/show_test.cgi?id=7755
 
> > > 13) the snippets (first line of body) require gloda to be enabled.
> > 
> > dito ?
> 
> W/o gloda, no snippets.


https://litmus.mozilla.org/show_test.cgi?id=7754
Comment 89 Phil Ringnalda (:philor) 2009-05-28 10:43:18 PDT
Comment on attachment 379990 [details] [diff] [review]
[checked in] patch for review v8

>diff --git a/mail/base/content/multimessageview.xhtml b/mail/base/content/multimessageview.xhtml

>+                    <hbox id="buttonhbox" align="start">
>+                        <button id="archive" class="multimessage button"
>+                                onclick="window.top.MsgArchiveSelectedMessages(null)">&archive.label;</button>

"Warning: XUL box for hbox element contained an inline #text child, forcing all its children to be wrapped in a block."

I suspect you meant label="&archive.label;".
Comment 90 David Ascher (:davida) 2009-05-28 10:58:56 PDT
Created attachment 380195 [details] [diff] [review]
fix for xul warning [checked in]

thanks for catching that, phil.
Comment 91 Phil Ringnalda (:philor) 2009-05-28 14:49:38 PDT
Comment on attachment 380195 [details] [diff] [review]
fix for xul warning [checked in]

Yup, thanks.
Comment 92 Phil Ringnalda (:philor) 2009-05-28 20:52:16 PDT
Comment on attachment 380195 [details] [diff] [review]
fix for xul warning [checked in]

http://build.mozillamessaging.com/mercurial/comm-central/rev/0863fd9a6154
Comment 93 [:Aureliano Buendía] 2009-05-29 02:09:37 PDT
I think that the new feature has broken the usability of the splitter on message pane: I cannot move message pane to top or bottom. This happens also if I dont have select any collapsed thread.
Comment 94 Mark Banner (:standard8, afk until Dec) 2009-05-29 02:12:26 PDT
(In reply to comment #93)
> I think that the new feature has broken the usability of the splitter on
> message pane: I cannot move message pane to top or bottom. This happens also if
> I dont have select any collapsed thread.

The splitter works fine for me with any combination of messages selected or not selected at all:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090528 Lightning/1.0pre Shredder/3.0b3pre
Comment 95 [:Aureliano Buendía] 2009-05-29 02:23:36 PDT
Restarting TB also hre work fine. I'm not able to reproduce now.

I try later.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090528 Lightning/1.0pre Shredder/3.0b3pre ID:20090528032146
Comment 96 Andreas Nilsson (:andreasn) 2009-05-29 07:30:24 PDT
Created attachment 380427 [details]
mockup v4

Realized that we can actually move the number of messages number indicator after the message, if we make the backgrounds for threads use a special color.
This would also give us more space for title, author, tags and message summary for all the other messages.
Comment 97 Ludovic Hirlimann [:Usul] 2009-05-29 07:39:36 PDT
(In reply to comment #96)
> Created an attachment (id=380427) [details]
> mockup v4
> 
Is the red stroked circle suppose to be the trash ?
Comment 98 Andreas Nilsson (:andreasn) 2009-05-29 07:48:10 PDT
> Is the red stroked circle suppose to be the trash ?

Yes, it will use the same icon as it does now (so don't worry, no change in metaphor).
People keep wondering about this, so if I do another mockup, I'll make sure to include the right icon..
Comment 99 [:Aureliano Buendía] 2009-05-29 08:48:24 PDT
Archive and Delete buttons not have the same height: it is desired behavior?

using

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090529 Lightning/1.0pre Shredder/3.0b3pre ID:20090529032206

attached file for details
Comment 100 [:Aureliano Buendía] 2009-05-29 08:49:16 PDT
Created attachment 380445 [details]
Buttons aren't with same height
Comment 101 David Ascher (:davida) 2009-05-29 10:07:30 PDT
Aureliano: Can you make a separate bug about that?  I see the same thing in the message header in XP, but I understand that may not be true in Vista.
Comment 102 [:Aureliano Buendía] 2009-05-29 10:22:29 PDT
Ok, that's bug https://bugzilla.mozilla.org/show_bug.cgi?id=495478
Comment 103 David Ascher (:davida) 2009-05-31 00:00:38 PDT
Created attachment 380685 [details] [diff] [review]
patch making things faster and prettier

This patch does a few things:

1) uses a variation of andreas's css, which works much better, and is much prettier to boot.  I was getting sick of getting lost in the DOM manipulations, so I used e4x to template the message rows, which works reasonably as an alternative.

2) figures outs everything from the selected messages w/o requiring gloda, although it still kicks of a gloda collection and uses that to get notified of changes to the messages (tagging, stars, read/unread status).  This required a bunch of refactoring which turned out well.

It's not quite review ready yet because there's an XXX there which is a question to Andrew about how we want to refactor the logic that asks gloda attribute providers to do the whittling (see mimeMsgToContentSnippet).  For now I just copied the function mostly as is, but it's clearly not the final refactoring.  

Note: there is a possible problem (whether w/this bug or without) if a single message in a collapsed thread among multiple thread selections changes, where gloda won't tell me enough about the change to know accurately what tags to display.  Given the rarity, and the lack of pernicious effects, I decided to live witht that.

Apologies to localizers, but after talking to bryan we simplified the heading text, which meant a string change.

One design question for Andreas/Bryan is how to display the messages in the right place with a header that can grow if the subject is super super long.  The current setup works ok for 1/2 lines, but at 3 lines fails a bit.  Maybe that's ok?  (oh, also I can't get the individual tags to wrap, don't know why). 

David: with this patch, the failure to stream (bug 495304) still occurs for me, with imap folders and local folders both, which should make it easier to track down I hope (no gloda bug masking that one).

I haven't looked at this patch on windows or linux yet -- would appreciate css comments if anyone does try it before I get there.
Comment 104 David Ascher (:davida) 2009-05-31 00:01:42 PDT
Created attachment 380686 [details] [diff] [review]
_correct_ patch

I'd forgotten to refresh.  This patch passes the test suite and looks nicer.
Comment 105 Mark Banner (:standard8, afk until Dec) 2009-05-31 05:35:47 PDT
*** Bug 495649 has been marked as a duplicate of this bug. ***
Comment 106 David Ascher (:davida) 2009-06-01 10:48:40 PDT
Created attachment 380861 [details] [diff] [review]
Patch including connotent refactoring

I'd like asuth to review the gloda/connotent refactoring, and he may as well look at the other changes as I know he's been dealing w/ this code in his front-end patch.  bienvenu, sounds ok?
Comment 107 David :Bienvenu 2009-06-01 12:52:37 PDT
sounds fine, since connotent sent me to the dictionary anyway :-)
Comment 108 Andreas Nilsson (:andreasn) 2009-06-01 15:23:31 PDT
Created attachment 380930 [details]
mockup v5

David wanted to figure out what we should do with attachments in the mockup.
This version differ between single and multiple messages.
* 1 attachment, big mimetype icon.
* 2 or more attachments, small mimetype icons (a maximum of 4 though)

This mockup also include a line around threads, instead of a colored area (looked inactive, and gray text was invisible)
Comment 109 Ronald Killmer 2009-06-01 17:36:30 PDT
(In reply to comment #71)
> even w/o your tweak, we're starting the load of the new message before calling
> summarizeSelection; I guess the issue is that loading is async, and switching
> to the message pane is sync. We'll just have to see if it's an issue for anyone
> with release builds but I certainly wouldn't hold up this patch for it.

I am seeing an odd behavior related to this issue.  Used with a news thread where the first post in the thread has an inline image attachment. The summary was initially displayed as designed. What happened next was wrong. I clicked the next message in the thread pane, a single posting with no replies.

What displayed was the Image from msg one of the thread previously summarized. The image did not go away to alow the new selection to display. I had to navigate away to another message and come back to read the single posting.  If I move beck and forth between the first thread and the single posting the Image of msg one of the thread was flashing in the message pane before the posting was displayed. This was more than a flicker, perhaps 1/4 second before the repaint replaced the message pane content.

OS is Vista Win32, and build tested the 06/01/2009 nightly.
Comment 110 martin.guillon 2009-06-02 08:41:30 PDT
First i want to say how happy i am to see this new feature. That was the one thing i was missing from mail.app.

Now there is one thing really bugging me which is the sort order. In mail.app most recent mail appears first which is very cool when the conversation has more than 100 mails....
WOuld it be possible to be able to choose the sort order in the threads and consequently in the summary?
May be i should create a feature request for it?

Great work anyway
Comment 111 David Ascher (:davida) 2009-06-02 08:52:55 PDT
Martin: thanks for the kind words. Do file a bug, and mark it blocking on this one.  Feel free to cc: me to your bug.
Comment 112 martin.guillon 2009-06-02 09:04:33 PDT
i did(bug 495965) thanks a lot David !
Comment 113 Bryan Clark (DevTools PM) [:clarkbw] 2009-06-02 11:42:44 PDT
Comment on attachment 380930 [details]
mockup v5

This looks really good.

I like the outline for threads, that is a nice improvement.  

The attachment icons looks pretty good.  I'm wondering if we should overlay our generic 'attachment icon' on the right top of the mime-type icons to give the hint that these are attached.

Also all these different systems are looking fairly similar I'm wondering if we should be using different colour header backgrounds for each.  Something to give a quick visual cue that you're looking at a selection of messages vs. a thread summary vs. account central.

Nice work!
Comment 114 Mark Banner (:standard8, afk until Dec) 2009-06-02 15:02:00 PDT
I think we said this could do with a dev doc due to the fact the message pane is now within an extra vbox, adding dev-doc-needed keyword.
Comment 115 David Ascher (:davida) 2009-06-02 15:23:59 PDT
Created attachment 381166 [details] [diff] [review]
revised patch w/ thread circling and css splitting

1) use the rounded border around threads
2) change the connotent refactoring to return the meta dict, so that things like the gp-bugzilla plugin can send data back to the summary viewer.  
3) move some of the css to a sharedsummary.css file, which will be used by the folder/account summaries.
Comment 116 Andrew Sutherland [:asuth] 2009-06-02 17:16:10 PDT
Comment on attachment 381166 [details] [diff] [review]
revised patch w/ thread circling and css splitting

>diff --git a/mail/base/content/selectionsummaries.js b/mail/base/content/selectionsummaries.js
>--- a/mail/base/content/selectionsummaries.js
>+++ b/mail/base/content/selectionsummaries.js
>+      // for tags, there's a minor problem in that if _some_ of the items in a
>+      // thread got modified 

>diff --git a/mailnews/db/gloda/modules/connotent.js b/mailnews/db/gloda/modules/connotent.js
>--- a/mailnews/db/gloda/modules/connotent.js
>+++ b/mailnews/db/gloda/modules/connotent.js

>+/**
>+ * Given a MimeMsg, return the whittled content string, suitable for summarizing
>+ * a message.
>+ *
>+ * @param aMimeMsg: the MimeMessage instance
>+ * @param folder: the nsIMsgDBFolder
>+ * @param length: optional number of characters to trim the whittled content.
>+ */

Please document the return value using the @returns tag.

Please mention the automatic inclusion of ellipsis when truncating when length is specified, and whether the ellipsis makes the total number of characters length or length+1.

>+function mimeMsgToContentSnippetAndMeta(aMimeMsg, folder, length) {
>+  let content = new GlodaContent();
>+  let meta = {subject: aMimeMsg.get("subject")};
>+  let bodyLines = aMimeMsg.coerceBodyToPlaintext(folder).split(/\r?\n/);
>+
>+  for each (let [, attrProvider] in Iterator(whittlerRegistry.getWhittlers()))
>+    attrProvider.contentWhittle(meta, bodyLines, content);

Please rename attrProvider; it made sense in its orginal context, but makes less sense here.

>+  if (length) {
>+    let text = content.getContentSnippet(length + 1);
>+    if (text.length > length)
>+      text = text.substring(0, length) + "\u2026"; // ellipsis
>+    return [text, meta];
>+  } 
>+  return [content, meta];

I find it confusing that if you don't specify length, you get the content object, which is definitely not a string.  Suggest making this method always return a string representation, but only truncating if length is specified and truncation is required.

I realize this change was made to avoid code duplication with the getMessageContent method, but it turns out weird.  Perhaps a better strategy is to also introduce a mimeMsgToContentAndMeta which this method calls?

>+WhittlerRegistry.prototype = {
>+  /** Add a provider as a content whittler.
>+  */

nit: Pleas format it like this:
/**
 * Add a provider as a content whittler.
 */

>+  getWhittlers: function whittler_registry_getWhittlers() {
>+    return this._whittlers.reverse();

Reverse is mutating (it just returns the same object).  The original code used blah.concat().reverse() to avoid rep exposure and mutation of the underlying list.

>diff --git a/mailnews/db/gloda/modules/gloda.js b/mailnews/db/gloda/modules/gloda.js
>--- a/mailnews/db/gloda/modules/gloda.js
>+++ b/mailnews/db/gloda/modules/gloda.js
>@@ -320,32 +320,8 @@
>+  getMessageContent: function gloda_ns_getMessageContent(aGlodaMessage, aMimeMsg) {
>+    return mimeMsgToContentSnippet(aMimeMsg, aGlodaMessage.folderMessage.folder);
>   },

I can't help but notice that this method does not actually exist.  I am therefore suspicious as to whether this passes the unit tests.  Please verify that the unit tests pass.
Comment 117 David :Bienvenu 2009-06-02 17:24:26 PDT
driving by (actually, running with the patch to try to track down bug 495304), I noticed some missing ; at the end of js lines...
Comment 118 David Ascher (:davida) 2009-06-02 18:00:00 PDT
Created attachment 381197 [details] [diff] [review]
review comments fixed

Thanks for the review.  This patch addresses all review comments, and the gloda test suite passes.
Comment 119 M.J.G. 2009-06-03 01:37:07 PDT
Trying the version currently on comm-central, I noticed:

When moving to an unread thread, the thread overview is displayed but the first message is marked read (even though it is clearly not being read). This has the adverse side effect that hitting 'N' (next unread) moves to the second message in the thread. In order to actually read the first one, one would have to expand it (right arrow).

Keeping the first message unread would solve all issues. I'm commenting here because the bug is still in progress. Please tell me whether I should rather file a new bug.
Comment 120 Mark Banner (:standard8, afk until Dec) 2009-06-03 01:59:20 PDT
(In reply to comment #119)
> Keeping the first message unread would solve all issues. I'm commenting here
> because the bug is still in progress. Please tell me whether I should rather
> file a new bug.

Take a look at list of bugs this one depends on and you'll find bug 495642 which already covers your issue.
Comment 121 M.J.G. 2009-06-03 02:23:26 PDT
(In reply to comment #120)
> (In reply to comment #119)
> > Keeping the first message unread would solve all issues. I'm commenting here
> > because the bug is still in progress. Please tell me whether I should rather
> > file a new bug.
> 
> Take a look at list of bugs this one depends on and you'll find bug 495642
> which already covers your issue.

Thanks for the hint and sorry for the noise.
Comment 122 David Ascher (:davida) 2009-06-03 09:42:01 PDT
Created attachment 381320 [details] [diff] [review]
works better with all the files

andreas pointed out i forgot a file.  i want hg qpostbz....
Comment 123 Andrew Sutherland [:asuth] 2009-06-03 12:46:24 PDT
Comment on attachment 381320 [details] [diff] [review]
works better with all the files

>diff --git a/mail/base/content/selectionsummaries.js b/mail/base/content/selectionsummaries.js
>--- a/mail/base/content/selectionsummaries.js
>+++ b/mail/base/content/selectionsummaries.js
>@@ -513,34 +527,66 @@
...
>+    for (let i = 0; i < this._msgURIs.length; ++i) {
...
>+      MsgHdrToMimeMessage(msgHdr, null, function(aMsgHdr, aMimeMsg) {
>+          if (aMimeMsg == null) /* shouldn't happen, but sometimes does? */
>+            return;
>+          let [text, meta] = mimeMsgToContentSnippetAndMeta(aMimeMsg, aMsgHdr.folder, 200)

nit, missing semicolon.

Thus concludes my delegated review.  gloda currently lives in mailnews though, so SR signoff required.
Comment 124 David :Bienvenu 2009-06-03 13:12:30 PDT
Comment on attachment 381320 [details] [diff] [review]
works better with all the files

sr=me on the gloda parts

one nit - this second line is a little over-indented:

+  if (length && text.length > length)
+      text = text.substring(0, length-1) + "\u2026"; // ellipsis
Comment 125 David Ascher (:davida) 2009-06-03 13:26:18 PDT
Created attachment 381364 [details] [diff] [review]
[checked in] nits addressed

carrying forward r+ and sr+
Comment 126 Mark Banner (:standard8, afk until Dec) 2009-06-03 13:56:53 PDT
Comment on attachment 381364 [details] [diff] [review]
[checked in] nits addressed

Checked in: http://hg.mozilla.org/comm-central/rev/81998e37543a
Comment 127 M.J.G. 2009-06-04 00:59:01 PDT
Hmm. With 81998e37543a I get the following new behaviour when moving (mouse click or key navigation) to the first message in an unread collapsed thread:

- The new style head pane appears with the thread subject and message count.
- The message pane (thread summary) lists the first message's author only.

If more messages in the thread are read then the summary lists more authors (not necessarily those of all read messages).

The same effect happens on old read threads. Once I reread those messages the summary page lists all authors but no snippets. I get snippets only for e-mail, not for news. Similarly, the issue above with missing authors seems to appear for news only.

This was different without 81998e37543a.
Comment 128 Richard Marti (:Paenglab) 2009-06-04 11:31:28 PDT
I can confirm Comment #127.

I am also unable to read the first message, when I click on the sender in the summary. This works only after I read this message and then collapse the thread and click on the sender again.
Comment 129 David Ascher (:davida) 2009-06-04 12:13:26 PDT
Created attachment 381577 [details] [diff] [review]
workaround to deal w/ remote messages (esp. news)

I hadn't taken into account the fact that MsgHdrToMimeMessage will throw an exception when faced w/ a remote message.  This is a workaround, but I wonder if we want a different API to said function.
Comment 130 David Ascher (:davida) 2009-06-04 12:18:14 PDT
argh, didn't mean to include the debug dump stuff.  ignore that stuff for review.
Comment 131 Andreas Nilsson (:andreasn) 2009-06-08 11:08:31 PDT
Created attachment 382159 [details] [diff] [review]
some css fixes

Here are some small css fixes. Put a gradient on the header (the color is switchable, so it would be easy to change if we went with the idea of different colors for different views), made the thread summaries slightly nicer looking and changed the objects using the inactive color in gnomestripe to use gray instead.
Not sure what pieces belong in the theme vs. everywhere, but the header might be something we could move to theme-specific places if it turns out it looks out of place on the other platforms.
Comment 132 Olli Pettay [:smaug] 2009-06-11 05:03:52 PDT
The current UI is strange. All I want to see is the first message in the thread to figure out whether I'm interested in the message thread.
When I first got the new UI (which contains the subject, the name of the sender and the size of the messages) I thought there was some strange bug.
Then I almost randomly clicked something and figured out that clicking the
expand-the-thread-button allows me to see the message.

As mentioned in the comment 0 in this bug, please, please include the text of 
the first message in the "thread summary" page.

For now I use mail.operate_on_msgs_in_collapsed_threads=false.
Comment 133 Mark Banner (:standard8, afk until Dec) 2009-06-12 03:55:25 PDT
Comment on attachment 381577 [details] [diff] [review]
workaround to deal w/ remote messages (esp. news)

I found several instances where this patch didn't work, for instance, have a collapsed unread thread where the bodies haven't been downloaded. Select it once, and you'll get "the message body hasn't been downloaded yet", select a different message then the collapsed thread again and you won't get any information.

Talking with asuth on irc:

asuth: er, un-ohhhh. but I was thinking bienvenu's new code might change our behavior there, perhaps
asuth: like, the first time you tried to cause this bug to happen, it said the right thing
asuth: but then the activeStreamListeners table got the URI put in it
Standard9: yeah second time
Standard9: it doesn't display the fact its not been downloaded
Standard9: and it runs that code with no exceptions and doesn't call the callback
asuth: right, because its callback is getting added to the list of dudes listening on that URI in activeSTreamListeners
asuth: Here is what I think:
asuth: 1) we should fix that bug where I am lazy about the unit test
asuth: 2) that code should be r-'d for using a catch-all exception as a standard control-flow concept
asuth: it should check the result of the exception to verify that it is the exception that streaming produces for things that are not offline
asuth: or better yet, the signature for MsgHdrToMimeMessage should do that itself and have a flag that says whether you want to force the streaming, etc.
Comment 134 Andrew Sutherland [:asuth] 2009-06-12 04:01:06 PDT
The bug I was referencing was bug 478167, which is what stops the JS Mime Emitter from working on news messages.
Comment 135 Ashley Bischoff (blog at handcoding.com) 2009-06-15 14:14:35 PDT
(In reply to comment #79)
> (From update of attachment 379990 [details] [diff] [review])
> Checked in: http://hg.mozilla.org/comm-central/rev/3c87898162a3
> 
> Note that I noticed the line endings in mail/themes/qute/jar.mn were messed up,
> so I fixed those.

This checkin may have regressed bug 498227.
Comment 136 Dan Mosedale (:dmose) 2009-06-29 21:36:58 PDT
As far as I can tell, the feature patches that were being tracked by this bug are in the tree and work, meaning that this bug is effectively now a meta bug.  We don't generally block on meta bugs, so setting blocking- and wanted-.  Please nominate individual dependent bugs to block if appropriate.
Comment 137 Mark Banner (:standard8, afk until Dec) 2009-06-30 00:53:54 PDT
(In reply to comment #136)
> As far as I can tell, the feature patches that were being tracked by this bug
> are in the tree and work, meaning that this bug is effectively now a meta bug. 
> We don't generally block on meta bugs, so setting blocking- and wanted-. 
> Please nominate individual dependent bugs to block if appropriate.

There is still ongoing work in this bug - we need the patch to deal with remote messages (i.e. newsgroups), as otherwise this makes the summary pane pretty much useless in newsgroups. As bug 478167 moved out to b4, I think we should try and get this in b3, especially as we have a skeleton of a patch anyway.
Comment 138 martin.guillon 2009-07-01 00:42:15 PDT
i thought i should mention how Postbox does it. Actually i think they found the perfect way to go. 
A few things i love about their approach:
- they dont indent messages in the message list. Very useful when you have more than a 10 messages. With indentation you just dont see anything anymore.
- In the message list, messages from the current selected conversation have a blue background. Very useful
- Their summary allows you to read message in the same way Gmail does. Very nice.
- Maybe the most important point for me, they order messages the other way around. Here i have newest mail on top, and in a conversation the newest message is also on top. I miss that a lot in Thunderbird.

I really think you should see how postbox does it if you didnt already. I really would love to see some of postbox features in thunderbird.
www.postbox-inc.com
What do you think?
Comment 139 Alfred Kayser 2009-07-01 04:15:33 PDT
On the last point fo comment 138: Just click on the 'Date' label of the data column to sort on Date, with newest on top.
Other points:
ad 1: could be implemented as a preference, because I like (and need!) the indenting: create a new bugzilla entry for this.
Ad 2: Another feature, that also requires a separate bugzilla entry.
Ad 3: Not clear what you precisely are looking for? Anyway, each new feature requires a bugzilla entry.
Ad 4: Existing functionality.

Please don't use this bug or bugzilla as a discussion forum. If you want new features, first check if the application doesn't already provide it, then check if there is already a bugreport for it, and then if there is no bugreport, create one with a clear description of the request.
Comment 140 martin.guillon 2009-07-01 05:27:16 PDT
sorry but it was not a feature request. I just thought it should be good for people who are developping this great feature in thunderbird, to know how it s down on a very similar software.
I wont comment more except for one thing. 
Sort on date is not what i am looking for. What i am looking for is for the order within a thread to be the inversed of the order within the list.
You can see that in mail.app and postbox. I already created a feature request for that
Comment 141 David Ascher (:davida) 2009-07-02 13:20:11 PDT
Martin: I heard your request to be able to sort the thread by the inverse chronological sort order from what we have here. I'll see if I can come up with a good way to do that.
Comment 142 Dan Mosedale (:dmose) 2009-07-07 08:59:40 PDT
After discussion with davida, removing blocking+ from this bug, as the last concrete work represented here has been spun off into bug 502839, which is a blocker.
Comment 143 Thomas Stache 2009-07-09 09:01:58 PDT
(In reply to comment #141)
> Martin: I heard your request to be able to sort the thread by the inverse
> chronological sort order from what we have here. I'll see if I can come up with
> a good way to do that.

I think what Martin said sounds like bug 478989, a conversation view.
Comment 144 raoul bhatia 2009-07-28 08:47:56 PDT
what about bug 474199?
Comment 145 Ashley Bischoff (blog at handcoding.com) 2009-07-28 11:45:09 PDT
(In reply to comment #136)
> As far as I can tell, the feature patches that were being tracked by this bug
> are in the tree and work, meaning that this bug is effectively now a meta bug. 

Sorry if I'm a bit slow on my response to this -- given that the quoted comment was from June 29 -- but while the feature patches being tracked by this are in the tree, they don't all work.

As mentioned in comment #135, this changeset (http://hg.mozilla.org/comm-central/rev/3c87898162a3) which was checked in as of comment #79, broke keyboard accessibility for deleting messages within threads (bug 498227).
Comment 146 Mark Banner (:standard8, afk until Dec) 2009-07-28 12:08:00 PDT
(In reply to comment #145)
> As mentioned in comment #135, this changeset
> (http://hg.mozilla.org/comm-central/rev/3c87898162a3) which was checked in as
> of comment #79, broke keyboard accessibility for deleting messages within
> threads (bug 498227).

You don't need to comment about that here. Just set it as blocking this one (as I am doing) and it will be tracked - it is filed so we know about it.
Comment 147 raoul bhatia 2009-07-28 23:29:53 PDT
Q: should "summary" a) be taken literally as in "condensed presentation" or b) as the thunderbird feature which previews multiple emails in the preview-pane?
Comment 148 Nomis101 2009-10-17 03:51:15 PDT
Yes, I know, this is an old bug, but I think you misspelled something ;-)
http://mxr.mozilla.org/comm-central/search?string=David+Ascerh
Comment 149 foudfou 2012-11-21 02:21:42 PST
Comment on attachment 381364 [details] [diff] [review]
[checked in] nits addressed

+      let msgContents = <div class="row">
+                          <div class="star"/>
+                          <div class="header">
+                            <div class="wrappedsubject">
+                              <div class="author">{author}</div>
+                              <div class="subject link">{subject}</div>
+                              <div class="count">{countstring}</div>
+                              <div class="tags"></div>
+                            </div>
+                            <div class="snippet"></div>
+                          </div>
+                        </div>;
...
+      let msgContents = <div class="row">
+                          <div class="star"/>
+                          <div class="header">
+                            <div class="wrappedsender">
+                              <div class="sender link">{senderName}</div>
+                              <div class="date">{date}</div>
+                              <div class="tags"></div>
+                            </div>
+                            <div class="snippet"></div>
+                          </div>
+                        </div>;

This is weird

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