115689 - Labels: sorting the label column is displayed in reverse of spec'd order

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[laurel]

Using dec17 commercial trunk build

When sorting on the Label column the order in which the sorted label groups
appear is opposite what is stated in the spec (unless I'm reading it
incorrectly). In either case of sorting with the labels at top or bottom, the
Red/Important should appear a top or bottom of list respectively.

Spec: http://www.mozilla.org/mailnews/specs/labels/#Thread

Excerpt from spec:
"Clicking on the Label column heading should toggle between sorting the messages
with labels at the top of the Thread Pane and sorting messages with Labels at
the bottom of the thread pane. Sorted at the top: Hottest, Hot, Warm, Cold,
Coldest, No Label. Sorted at the Bottom: No Label, Coldest, Cold, Warm, Hot,
Hottest. "

1.  Apply at least one of each label to a folder.
2.  Click the label column so that the labeled messages appear at the top of the
thread pane.
    Result:  labeled messages appear from top of thread pane in groupings in
this order (using default text/color): Later/Purple, To Do/blue, Personal/Green/
Work/orange, Important/red.
    Expected order after sort labels at top: Important/red, Work/orange,
Personal/Green, To Do/blue, Later/Purple.

Also in reverse order when sorting labeled messages at bottom of thread pane.

Comment 1

[Sean Su]

It actually sorts in the correct direction/order for me given the spec.  When
sorting in ascending order (triangle points down), I get the following order:
  none
  Important (red)
  Work (orange)
  Personal (green)
  To Do (blue)
  Later (purple)

in descending order (triangle points up):
  Later (purple)
  To Do (blue)
  Personal (green)
  Work (orange)
  Important (red)
  none

The only problem I see is that the messages without labels (none), is in the
wrong place.  This is because 'none' has an index value of 0, while the other
labels have values > 0.

However, I think that sorting in ascending order should be the opposite of what
it is now because I understand 'ascending' order to be lowest priority ->
highest priority and vice versa for 'descending' order.

If the ascending/descending orders were fixed, it would take care of the 'none'
label problem automatically.

Comment 2

[laurel]

I read the spec differently ... sort of like the Red/Important would always
appear as a "bookend" in that it would be at top or bottom of the list. I don't
want to see the least label level at the "bookend" at top or bottom... sort of
like date sorting puts the newest messages (most recent/most important
generally) at the very top or very bottom of the pane list.

Comment 3

[Sean Su]

Looks like what we need to do is redo the labels id-text mapping around a
little.  The current mapping is:
 back end id | front end description
           0   none
           1   Important (red)
           2   Work (orange)
           3   Personal (green)
           4   To Do (blue)
           5   Later (purple)

 back end id | front end description
           0   none
           1   Later (purple)
           2   To Do (blue)
           3   Personal (green)
           4   Work (orange)
           5   Important (red)

This way it will sort as you had expected (Important being at the 'bookend'). 
This fix should be done asap because it will reverse current labeled messages in
people's mail folders (Important -> Later; Work -> To Do; etc...).  The longer
we wait, the more people will be affected.

What do you guys thing of this change/fix?  The order in the labels menus would
also change accordingly.

Comment 4

[scottputterman]

We probably still want Important to be the 1st label.  Do we have to sort by
label id?  Can't we make it so that Label 1 is considered to be > Label 2, Label
2 > Label 3, and so on without changing the id?

Comment 5

[Sean Su]

David, I talked to scott a bit more.  He was wondering if it was possible to
change the sort for the labels column to sort the id's 1-5 as it is, but have 0
be greater than 5 (or always last in case we add more labels).  This way id 1
(current Important - Red) would be at the 'bookend' when sorted.

Jennifer, what do you think about the current sorting of the labels?  Should we
change the way the labels are set (see my comment #3 above), change the sorting
algorithm (as scott has indicated), or just leave as is and change the spec to
reflect the current sorting?

Comment 6

[David :Bienvenu]

yes, Sean, it's trivial to do whatever you want.

In nsMsgDBView::GetLongField, in case nsMsgViewSortType::byLabel, you can
perform whatever gymnastics you want on the label value to make whatever sort
order you want. If you look at the nsMsgViewSortType::byPriority case, you'll
see an example of how we change the sorting of priorities.

Comment 7

[jglick]

I like Scott's suggestion. Sort the id's 1-5 as it is, but have 0 be greater 
than 5 (or always last in case we add more labels).  

This will made it easy in the future if we want to add more Labels. Also even 
though, we are using "0" to represent "None", i think users think of "None" as 
outside/beyond thoses msgs with Labels. So >5 seems appropriate. 

Comment 8

[Sean Su]

Attachment: patch to fix sorting

This patch will fix the labels sorting order so that 1-5 are sorted as normal,
while 0 (no label) will be assumed to be 'last label + 1', so that it will
always be sorted last.

Comment 9

[racham]

I would say
(nsMsgViewCommandType::lastLabel - nsMsgViewCommandType::label0) + 1;
looks better. No big deal though.

r=bhuvan.

Comment 10

[David :Bienvenu]

Comment on attachment 62432 [details] [diff] [review]
patch to fix sorting

sr=bienvenu

Comment 11

[Sean Su]

patch checked in.

Comment 12

[laurel]

OK using jan09 commercial trunk: win98, linux rh6.2
OK using jan08 commercial trunk: mac OS X