Wrong sorting - please help me debug this

Mark Edwards mark at antsclimbtree.com
Thu Jul 27 04:35:37 CEST 2006


I found that I also had to comment the lines below them, like so:

     // if not already sorted
     if (!$headers_sorted)
       $a_msg_headers = iil_SortHeaders($a_msg_headers, $this- 
 >sort_field, $this->sort_order);


//      if (!$headers_sorted && $this->sort_order == 'DESC')
//        $a_msg_headers = array_reverse($a_msg_headers);


Perhaps that's because I am using an older trunk, from a week ago or  
so.  Anyway, just my 2 cents.

I agree with Eric, why not respect the sort that the IMAP server  
performed?  Shouldn't that be authority?  Or at least have it  
configurable (allow server-side sorts, like with Squirrelmail).

On Jul 26, 2006, at 8:48 AM, Eric Stadtherr wrote:

> Thomas,
>
> That's essentially what I did in my fix - use the sequence returned  
> by the IMAP SORT command to re-order the list returned by the FETCH  
> command.
>
> The only problem with using iil_SortHeaders to sort the fetched  
> list is that it implements its own sorting algorithm, ignoring the  
> sequence returned by the SORT command. The algorithm probably  
> produces the same results, but why tell the IMAP server to sort the  
> list and then throw away the results? I think we need to choose one  
> or the other - tell the IMAP server to sort the list, or execute  
> our own sorting algorithm. I vote for letting the IMAP server do  
> the sort - this automatically gives us support for any future IMAP  
> sorting behavior without having to maintain our own algorithms.
>
>  Thoughts?
>
>
> On Wed, 26 Jul 2006 09:23:16 +0200, "Thomas Bruederli" wrote:
>
> Hi,
>
> Actually RoundCube should attempt to sort the message set retrieved
> from the server again because you can't be sure that the messages are
> returned in the right sequence. This is done in rcube_imap.inc on line
> 571/572 but those lines were commented out in revision 203 by myself
> (shame on me!). Making use of iil_SortHeaders() again should solve the
> problem. If not, we should try to improve the sorting algorithm in
> that function.
>
> Regards,
> Thomas
>
>
> 2006/7/26, Eric Stadtherr :
> >
> > All,
> >
> > I found the problem and have completed the fix. Essentially what  
> was happening is that the "SORT" command to the IMAP server would  
> return the message sequence numbers in the correctly sorted order.  
> RC would then perform a "FETCH" command to get the message header  
> data, using the ordered sequence numbers that came back from the  
> SORT Unfortunately, the IMAP server is free to ignore the order of  
> the sequence numbers provided in the FETCH command (see RFC 3501  
> §9, "sequence-set"), so it was returning the headers in ascending  
> sequence number order (ignoring the sorted order). The subsequent  
> RC code assumed that the FETCH command returned the messages in the  
> requested order, causing the incorrect order in the display.
> >
> > I may have overengineered the fix, but I haven't noticed any  
> performance hit at all. I put the patch at:
> >
> > http://stadtherr.bounceme.net/files/rc_sort_fix.patch
> >
> > Someone will have to let me know how best to get the fix  
> incorporated (I don't have SVN access).
> >
> > Enjoy!
> >
> >
> >
> > -Eric
> >
> >
> >
> >
> > PS. I haven't tested it with caching turned on, since I like to  
> leave that off If someone has caching enabled and wants to test  
> this, can you let me know how it works?
> >
> >
> >
> >
> > On Mon, 24 Jul 2006 22:44:46 -0600, Eric Stadtherr  wrote:
> >
> >           I found the problem and I have a patch almost ready.   
> My fix is working for small folders but it doesn't handle multi- 
> page message lists yet.  I'll post it as soon as it's done!
> >
> >  -Eric
> >
> >
> >  Eric Stadtherr wrote:
> >
> >
> > I did some digging into this issue over the weekend, and  
> discovered that rcube_imap::_list_headers is getting a correctly  
> sorted list of message UIDs back from iil_C_Sort().  This means  
> that the sorting within the IMAP server (mine is dovecot) is being  
> done correctly, but RC is still displaying them in an incorrect  
> order (the order of UIDs in the display table doesn't match the  
> order of UIDs that come back from iil_C_Sort().  I didn't get a  
> chance to trace it any further through rcube_imap::_fetch_headers  
> and out through the display code.
> >
> >
> >
> > On Mon, 24 Jul 2006 09:49:23 -0700, Mark Edwards wrote:
> >
> >      On Jul 24, 2006, at 9:17 AM, Jim Pingle wrote:
> >
> > > There appears to be support for a similar option in the code for
> > > roundcube
> > > (in imap.inc), but I see no method for a user/admin to set it. In
> > > rcube_imap.inc it's explicitly set off, but at the start of
> > > imap.inc it is
> > > set on. It tests to see if IMAP_USE_HEADER_DATE is false which it
> > > always
> > > will be as I can't find any other reference to the variable it's
> > > testing for
> > > anywhere in the code.
> > >
> > > As an experiment you could try commenting out line 57 in program/
> > > lib/imap.inc:
> > > if (!$IMAP_USE_HEADER_DATE) $IMAP_USE_INTERNAL_DATE = true;
> > >
> > > Alternately, change that so it's set to false instead of true.
> > >
> > > If it alters the sorting behavior for you, we may have found the
> > > culprit...
> >
> > Unfortunately, that did nothing.  :-(
> >
> > Squirrelmail get the sort right on this particular mailbox, whether
> > server-side sorting is enabled or not.  Roundcube puts a bunch of  
> old
> > mails out of order at the top of the box.
> >
> > --
> > Mark Edwards
> >>


--
Mark Edwards


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.roundcube.net/pipermail/dev/attachments/20060726/89749968/attachment-0001.html>


More information about the Dev mailing list