paging long threads

It will not really solve the issue, but can you test the two performance patches at #372201-13: Roadmap for official 1.0 release?

I had a patch doing this somewhere in the issue queue... cannot seem to find it. It will most likely need to be rerolled and also need to add more logic to open the thread to the correct page though.

Ok, attached is a first patch. It's still in progress, but should work.

What it does:
- Basically, it works similar to facebook's thread view. It just shows you the latest X (currently hardcoded to 10, see follow up) messages if you have more than those.
- If you do, it displays a link which allows to see all messages. Once you do that, there is no paging anymore (see todo)
- Does fix some other related and necessary things. for example, it converts the DISTINCT to a GROUP BY in the messages query so that the count query works.

TODO:
- configurable X
- Don't display all messages, just display X more
- Always display unread messages. (This one is tricky, I'm not yet sure how to do this).
- Remove the having, since that makes the query slower :)
- Maybe define a (configurable) hardlimit, so that no more than, say, 100 messages are displayed at the same time(you could still display the first 100 messages of a 150-message thread)

Please test this and report bugs!

Attachment	Size
privatemsg_paging.patch	5.27 KB

THis bit seems weird:

you are checking if there is a $limit after checking for it already.

Also, this seems to be more complicated than just using a pager query or a db_query_range - what are the benefits to this approach?

Yes, that second $limit should be $account, thanks.

The idea is that we want the *last* X messages (but still sorted in the old way). db_query_range() can't do that directly. However, I just had an idea how to do that based on the count query. Will work on a new patch after RC4 release.

Oh, and usual question, can you join #drupal-games? I'm preparing the RC4 release and looking for things that need to be fixed before that.

Updated the patch..

Changes:
- It does now use a usual db_query_range() which is based on the count query. This makes it a lot easer.
- It does now have a configurable hardlimit. Basically, the limit/show defines where it starts to display and the hardlimit defines how many messages are displayed at the same time. You can think of it as a sliding window through the message list.
- Added admin settings, not sure if it is understandable..

TODO:
- Improve "paging widget". Suggestions are welcome. Mabe something like "(display earlier) Displaying messages 20-120 from 180 (display newer)", while the (links) are only displayed when necessary.
- Handle new messages. I have no idea if/how to do that. Meaning, the paging window should be positioned where the first unread message is by default. Note that new messages are only marked as read when they are actually displayed.
- Maybe different default values, 100 might be too high for the hardlimit.

Attachment	Size
privatemsg_paging2.patch	5.79 KB

Something that can probably be added to this - currently there is a variable called "privatemsg_per_page" which is a braindead name.

"privatemsg_thread_messages" that is introduced in this patch is not as bad, but something to consider as both could easily be confused. (we also need a variable_del for it.)

privatemsg_threads_limit and privatemsg_view_limit could be alternative variable names... or maybe something else?

I still think if we could do either lazy loading or message collapsing a'la gmail, that would be a better option than paging.

@nbz: Of course, that would be nicer. But it would actually need both (gmail is doing lazy-loading, it only loads the content when you click on a colapsed message) and it would be a lot more complicated than the patch is currently doing because of our flexible API/hooks. For example, just loading a author/sent time list would not go through our hook_privatemsg_message_load hook which technically allows to change that information.

So, my plan is to add *something* to 1.0, because simply adding enough replies will kill the server and that's not nice. Then we can try to improve it in 2.x.

Updates:
- renamed variable names
- implemented back/forward paging widget
- handled the special case when $limit is greater than message_count
- Added some tests

I could use some help with the i18n aspects of that patch, mainly if it is possible to translate that into other languages easily (right to left languages and so on)

Attachment	Size
privatemsg_paging3.patch	9.77 KB

Updated, the previous/newer links are now inserted into the t()-String.

I'm still looking for feedback/reviews regarding this. This is the last feature I want to have in 1.0 so everyone is invited to try and test this patch so that we can release 1.0 soon.

Attachment	Size
privatemsg_paging4.patch	10.13 KB

Hi

It probably doesn't work right.
concrete examples:

At last page I have this paging "<< Displaying messages 42 - 51 of 51"
but when i click on "<<", the paging changed to "<< Displaying messages 32 - 51 of 51"

So far as I understand it I think that there could be 2 pagers, on on letf "<<" and one on the right ">>" (right is missing) and the text between could be "Displaying messages 32 - 42 of 51"

I am not sure if the pager << < 1 2 3 4 5 > >>would't be better and more usable. There will be good for sure to add paging not only on the top of the thread but on the bottom of the thread too.

There is one thing I am not understand completly - Hard display limit for messages inside threads
can you pleas more explain reason of it and how exactly it works?

btw I am using latest dev privatemsg version + this patch.
Thanks
Igor

There is one thing I am not understand completly - Hard display limit for messages inside threads

This is exactly what you are describing as a bug.

limit just defines how many messages to display initially and how many should be loaded each time you click "<<".

hard_limit defines how many messages should be displayed at the same time (maximally). So in your example, when you have 51 messages and hard_limit is set to 50, which is the default, you can click "<<" 4 times and then you are displaying message 2 - 51. The next time you click, hard_limit is actived and it only shows message 1 - 50.

The reason I implemented it like this is:
a) we start at the end of the thread so we can't (afaik) use pager_query() like other paged views.
b) It allows to view earlier messages without "loosing" older ones. (unless hard_limit is reached).

Hint 1: You can set hard_limit to the same value as limit and it will always only display that amount of messages.

Hint 2: This is not a perfect solution. I will most probably work on a solution that loads more messages with Ajax and ads them to the current page without a reload. This will also remove the need for an hard_limit. However, even with that, a solution that works without JS is still required.

I think by default we should have limit and hard limit to be the same value?

With a higher hard limit, I think the way of paging through pages that are the same (if that is the described behaviour) will confuse many users. I need to test this yet and may have some time in the next few days.

(just in case, I will mention here - there is another visual clean up I want to work on for 1.0 - having all new messages be physically marked as so in order to visually differentiate them from the read messages.)

I think by default we should have limit and hard limit to be the same value?

Yeah, we can do that. And we probably need a better description, can anyone suggest something?

(just in case, I will mention here - there is another visual clean up I want to work on for 1.0 - having all new messages be physically marked as so in order to visually differentiate them from the read messages.)

That would be great, I thought about that too. Any chance you can give a timeframe until you can provide a patch? a day? 3?

My suggestion:

- first page show 10 (by default) last messages;
setting name: "Number of messages on first page inside thread";
description: "Define how many messages should be displayed by default. It does display the last messages and earlier messages can be displayed on archive pages.";

- click to "<<" button show 50 (by default) last messages;
setting name: "Number of messages on archive pages inside thread";
description: "Threads will never display more messages than defined here at the same time. It is still possible to view all messages of a thread, but not at the same time.";

- double click to "<<" button show 50 first messages, if we have thread with 100 messages;

- etc.

Interesting feature: displaing only new messages + some old (1,2,3, configurable) on first page. How difficult is to make it?

I like that, I think. And it's actually not so different from the current implentation (technically) so attached is an updated patch.

Regarding your idea to display all new messages, that was my idea too, but it needs another query and more changes and I'd like to get this commited asap. Definitly something to consider for 2.0

Please test!

Attachment	Size
privatemsg_paging5.patch	10.53 KB

Works great for me. Thanks!

Sorry about taking so long with this... just a few things I have been thinking about.

...Unread messages will always be displayed...

Should this bit be deleted from the description?

There seems to be no show all option? That should preserve the current view for people who want it.

From what I understand... that is getting the maximum number of messages that can be shows according to the settings instead of what is actually requested. Is that a mistake?

Too verbose IMO. just "the default number of messages to be displayed per page." should be enough.

"Threads will not show more than this number of messages on a single page." ?

There seems to be no show all option? That should preserve the current view for people who want it.

We could probably add something like 0 or 'unlimited' and handle that specially, but, you can easily set both variables to a value that is high enough to display any amount of message. I would highly recommend against doing so however as it will use more and more memory.

From what I understand... that is getting the maximum number of messages that can be shows according to the settings instead of what is actually requested. Is that a mistake?

No, I don't think it is a mistake. LIMIT $startwith, variable_get('privatemsg_view_max_amount') means to start with the row $startwith (by default, COUNT(*) - 10) and load no more than max_amount, to avoid out of memory fatal errors. Clicking back/forward just updates $startwith, the max_amount always stays the same.

I will update the strings, agree about being too verbose. I thought the same actually, but forgot the change them.

Updated the patch.

I will commit this soon, unless someone has better ideas for the admin strings or does find a bug.

Attachment	Size
privatemsg_paging6.patch	10.38 KB

Ok, decided to go forward and commit this. Please report any bugs and ideas about this in new issues.

Thanks to everyone who helped testing.

Just a couple of quick suggestions from using it on a live site...

Selecting the numbers on the admin pages - would it be better to have a drop down like we have for number of threads?

I don't think we need two separate numbers for which to display. I find it confusing even though I know better. Maybe this is something that will be learnt.

Also, paging is not "consistent" - with default settings in a thread with 125 posts:

First page has messages 116 to 125,
Second page has messages 76 to 125,
Third page has 26 to 75
Fourth has 1 to 50.
The third now has 51 to 100 and the fourth 100+

This breaks visual consistency where you "know" when going through it one way that the message you are looking for is half way down the page... its position changes and jumps.

So, we should have some method where the default listing sticks to the "boundaries", such as the first page being 101+, second 51 to 100, third being 1 to 50, unless some manually puts in a specific start number of their choice.

Another thing to consider = making the displaying messages X of Y - making that a permanent presence (maybe even duplicated under the list of messages?)? If people are not used to seeing it there, and suddenly some messages are missing, it can take a while to reacclimatise. Some visual theming to differentiate it from its surroundings may be nice, but then it might not remain theme neutral by default.

Otherwise it works well, but I think it requires a tweak or two to make it simper and more consistent.

also, the new variables have not been added to the uninstall routine.

Selecting the numbers on the admin pages - would it be better to have a drop down like we have for number of threads?

I am not sure, probably yes. It might limit some very special uses cases, but you can still set the variable in your settings.php if you have to.

I don't think we need two separate numbers for which to display. I find it confusing even though I know better. Maybe this is something that will be learnt.

Hm, let's see if we get more feedback on this. I like it the way it is now, since you usually only want to see the last few messages so that you don't have to scroll that much when replying. But when displaying older messages, it is ugly if you have to click 25x times to get to your message you want to see.

Second page has messages 76 to 125,
Third page has 26 to 75
Fourth has 1 to 50.
The third now has 51 to 100 and the fourth 100+

This breaks visual consistency where you "know" when going through it one way that the message you are looking for is half way down the page... its position changes and jumps.

So, we should have some method where the default listing sticks to the "boundaries", such as the first page being 101+, second 51 to 100, third being 1 to 50, unless some manually puts in a specific start number of their choice.

I see. Imho, the way it is *when going back* is intended as it is now, but when you are on 1-50 and click ">>", it should display 26 to 75 again. Should be easy to fix.

Another thing to consider = making the displaying messages X of Y - making that a permanent presence (maybe even duplicated under the list of messages?)? If people are not used to seeing it there, and suddenly some messages are missing, it can take a while to reacclimatise. Some visual theming to differentiate it from its surroundings may be nice, but then it might not remain theme neutral by default.

Agreed, I will change it to be always there. Do you now any standard visual appearance (that is part of core, if possible) we could use? Maybe simply a framed box, or something like that. We do not necessarly have to be theme neutral, we aren't anyway, afaik (colors and stuff).

TODO:
- uninstall
- Always display pager
- Fix going back
- Add select with useful defaults (3, 5, 10, 20, 50, 100, 1000 (= unlimited))? Also, a warning when default size is lower than max size might be helpful since that would lead to unexpected results (newest messages are not shown)

Hi

IMHO, and it could be seen from my byg report what I actually I found that was feature, it is not very "understable" to have 2 numbers.
the first for visible messages on the page and the second for max number of messages on the page.

I would like to have an option to set number for max messages in the thread. That's all. So for me it will be enough to have this one number in settings.

Maybe the opinion from others would be different, this is how I can see this - one setting for number of messages in the thread. It is easy to understand it.

thanks
Igor

I thing better to reformat this

Yeah, ok, I've been convinced ;)

Changes:
- merge variables into a single setting
- uninstall variable
- Always display pager (above and below messages)
- Fix going back
- Add select with useful defaults (3, 5, 10, 20, 50, 100, 1000 (= unlimited))?

Please test the patch.

Attachment	Size
privatemsg_paging7.patch	10.77 KB

"3, 5, 10" = "useful defaults"? No, "3, 5, 10" = "lol". In a thread with 125 posts I will press a "<<" button 42-25-13 times for view a history. It's ugly decision.

Users want to see only few last messages on first page - 3, 5, 10 - but not 20. Users hate scroll page (to message form, to last message) on first page, but on archive page it is ok. So 2 numbers are very "understable" to me :)

Paging defaults highly dependable from site to site and thematic. Provided defaults seem reasonable for me.

@Scrolling issues - that is why we have named anchors. when you click the message title, if there are unread messages, a #new is appended to the url, taking you to the correct place. (pmgrowl can also be modified to link to the correct message instead of just to the page in question).

Another potentially useful special casing suggestion (in psuedo code): if $max_allowed < $new_count, $max_allowed = $new_count;

I would have zero for unlimited - that is also more of the convention I think.

As for theming conventions, I don't think there is any, and if there was, it may not suit the default message display.

I would keep the default amount variable name instead of max amount, I would also think it sensible to change the paging to use "start=" $_GET syntax for paging, and use "show" to override the default number of messages.

(if you want to keep both the max and the default, that is also ok, but then the help text will need to be changed to specify that the default is only for the first page, and not a default in the other sense of the word where it is the default on all pages.)

@Gyt: These defaults do not have any effect unless you choose them. What I meant with "useful defaults" are values, that can match almost every possible use case.

@nbz: We can easily show unlimited/0 and use 1000 as the value, assuming that every site will break anyway when you try to show 1000 messages. The simple reason is that the code is already full of nested if/else code because of all those special cases.

@andypost: I'm not sure what the difference would be there, for me, the links before/after the text are easier to understand. Other opinions?

@All: As andypost has mentioned, there are many, very different sites out there that use privatemsg in very different ways and I've always tried to be as flexible as possible. Any chance we can find a useful admin UI that would still allow having two variables without overcomplicating it? Maybe a (by default checked) checkbox "[x] Display max amount of messages by default" that would enable the second select if disabled?

Note to myself: Commiting not 100% finished stuff seems to help getting a discussion. Might make sense to commit some not 100% ready stuff to the 2.x branch once opened to get more testers/feedback than many issues currently have.

Update:
@nbz: Ok, a real unlimited/0 value might make sense if we then hide the pager....

that is why we have named anchors. when you click the message title, if there are unread messages, a #new is appended to the url, taking you to the correct place.

I know it, but "#new" don't work, if there aren't any unread messages. For example, user decide to add something to one's own answer and he go back to thread. Or he want to write comment (question) in old thread.

These defaults do not have any effect unless you choose them.

Oh, sorry, but 3, 5 or 1000(?) are really ugly for paging. Maybe get useful defaults from comment settings (10, 30, 50, 70, 90, 150, 200, 250, 300)?

Maybe a (by default checked) checkbox "[x] Display max amount of messages by default" that would enable the second select if disabled?

Maybe a (by default unchecked) checkbox "[x] Display another amount of messages on first page of thread"?

Once you guy are done bikeshedding on defaults of message pagering, anyone up for grabs to implement scroll based auto-pager js ? Similar behavior to facebook when you scroll down it loads more items to your wall. So with pmsg when you scroll down to the end of the page it would auto-load next page's messages and attach them to the end of the page thread. thoughts?

Ok, changes..

- Reintroduce second setting, which is only displayed if you check the checkbox "Display different amount of messages on first thread page"
- Updated the possible values, including an "unlimited" (only for max_amount). It is possible to choose unlimited and have a default_amount, it will simply display all messages once you click on the "<<" the first time.
- Added more tests

Attachment	Size
privatemsg_paging8.patch	16.82 KB

the listing is still awkward IMO.

A few issue:

1. Make the pager centered?
2. the last and the second to last pages will have some messages overlapping, so it is not totally consistent and easy to tell where they have read up to.
3. the "show" $_GET argument is badly named since to me that indicates the number of messages to show, and not how far back in the listing we are going. I think this needs to be renamed, but "start" may also be a bad substitute, since we will not be starting with message number X, but starting with a negative offset from the total number of messages.
4. the uninstall is only deleting one of the three new variables.

if we were not going backwards, a simple "traditional" pager may have been a decent option to start with before getting fancy, but too late for that now.

@litwol - we cannot copy facebook there as I presume their message listing is recerse ordered? ours isn't.

+ $amounts = drupal_map_assoc(array(5, 10, 30, 50, 70, 90, 150, 200, 250, 300));
+ $form['privatemsg_listing']['privatemsg_view_max_amount'] = array(
+ '#type' => 'select',
+ '#title' => t('Number of messages on thread pages'),
+ '#options' => $amounts + array(PRIVATEMSG_UNLIMITED => t('Unlimited')),
+ '#default_value' => variable_get('privatemsg_view_max_amount', 20),
+ '#description' => t('Threads will not show more than this number of messages on a single page.'),

So, maybe include "20" in drupal_map_assoc?

2. the last and the second to last pages will have some messages overlapping, so it is not totally consistent and easy to tell where they have read up to.

instead

+ elseif ($limit > $max_amount && $limit > $thread['message_count']) {
+ $limit = $thread['message_count'];
+ }

use

+ elseif ($limit > $max_amount && $limit > $thread['message_count']) {
+ $max_amount = $thread['message_count'] - ($limit - $max_amount);
+ $limit = $thread['message_count'];
+ }

instead

+ else {
+ $to = $from - 1 + (($thread['limit'] > $max_amount) ? $max_amount : $thread['limit']);
+ if ($to > $thread['message_count']) {
+ $to = $thread['message_count'];
+ }

use

+ else {
+ $to = $from - 1 + (($thread['limit'] > $max_amount) ? (($thread['limit'] > $thread['message_count']) ? $thread['message_count'] - ($thread['limit'] - $max_amount) : $max_amount) : $thread['limit']);
+ if ($to > $thread['message_count']) {
+ $to = $thread['message_count'];
+ }

It solve the problem.

So, maybe include "20" in drupal_map_assoc?

Erm, yeah...Either that or use a different default value. What do you think?

working on a new patch...

I think, include 20 is better choice.

While you're working ona aseparate patch - I would think it would be good to have the same number of messages as defaults in variable_get for the variables - if people want different, let them set it so themselves.

The never-ending story goes on...

1. Make the pager centered?

Done, including a fancy... erm, ugly border :) First try was centered, current version is technically not centered, because margin-left and with of the box is fixed now but almost looks as it is. Could still make the text centered inside the div. Not sure...

2. the last and the second to last pages will have some messages overlapping, so it is not totally consistent and easy to tell where they have read up to.

Changed, they don't overlap anymore. The last page just shows the rest now.

3. the "show" $_GET argument is badly named since to me that indicates the number of messages to show, and not how far back in the listing we are going. I think this needs to be renamed, but "start" may also be a bad substitute, since we will not be starting with message number X, but starting with a negative offset from the total number of messages.

Changed to start now, feel free to suggest better alternatives if you can think of any. I don't consider that to be important though... :)

4. the uninstall is only deleting one of the three new variables.

Fixed

Other chances
- More tests (They seem to be pretty good now, It took me ~10min to apply all stuff from above and ~1h to have all tests working again :p would be so ugly to catch all these little special cases without those)
- Moved all calculation stuff to the thread_load() functions, privatemsg_view() again just displays the things if they are defined.
- Added 20 to the possible values (thanks Gyt)

if we were not going backwards, a simple "traditional" pager may have been a decent option to start with before getting fancy, but too late for that now.

Maybe. I'm not so sure about that... I'm still hoping that my custom pager is easier to extend (new message handling, ajax fancyness) than the default one.

While you're working ona aseparate patch - I would think it would be good to have the same number of messages as defaults in variable_get for the variables - if people want different, let them set it so themselves.

I'm not so sure about that. I mean, they wouldn't tick the checkbox when they would want to have the same value... Note: The default_amount is *only* used if that box is checked! No magic involved...

Attachment	Size
privatemsg_paging9.patch	18.54 KB

Good enough to be committed IMO, however the following concerns remain:

1. As you mentioned, the theming is not the prettiest - but here there is a question of good defaults and having the theming in the nightly may get themers to take a look and suggest something. (but potentially, we could reduce the top and bottom margins, increase the passing... and maybe even give the box a 100% width.)

2. Make the paging even more consistent... by this I mean replace the start number to not be an offset - "start=20" should show the 20th message in the thread up top, not the 20th from the end. Reason being canonical links - with "live" threads, saving links/bookmarking is currently useless as the place that the link goes to will change with each additional reply to the thread.

3. Feature to show all unread messages on first page.

All three can (I think) wait for a new patch. apart from

Ok, I'll go forward and commit this then, It's for sure better than what we have currently in the -dev release.

1. I tried with a 100% width, didn't like how it looked.

2. I see what you mean and why you'd want that. However, it would need even more checks and nested ifs to be able to differentiate between the setting, which shows count - setting messages and the incoming $_GET['start'] value.

3. Probably not even necessarly show all of them but start where the first unread message is (probably 2-3 messages earlier, actually). I'd love to have this, but I'm sure it's not that easy, as always, there are many edge cases. For example, you only want that feature to be activated if not all unread messages are already on screen for the default list (it jumps to the #new message in that case) and requires probably different handling based on the default_amount setting and is also different when *all* messages of a thread are unread (you can't display older, read messages then). And so on...

Thanks for all the reviews so far, you are still welcome to test and report any bugs you can find, if possible, I'll fix them before 1.0 or if that's not possible, later on in a bugfix release.

I'll try to merge the two applied patches before porting to D7, we'll see how that goes.

Commited against 7.x-1.x-dev.

Some more ideas about how the pager should be further modified.

I replaced $limit with $start as it seems to be a more logical name. $start is now an offset from the start of the thread and I have added two sanity checks:

1. $start cannot be greater than the total message count.
2. $start cannot be lower than zero.

This probably needs improving yet though.

Attachment	Size
privatemsg_pager.patch	5.38 KB

Currently, the showed value is always one too low, you need to add 1 as I did. This is because the first message is #0 for mysql, but it's #1 for you. And if you do that, you have to re-add the - 1 to the db_query_range call.

Maybe even just pager? or pager_top? and then obviously pager_bottom...

Oh, and don't ask me wtf display_all stands for. I have no clue :)

Additionally, you seem to have removed the feature that going back/forward is consistent. I don't know if you just forgot it or if there is a reason but I doubt that. Example:
- First, it displays "<< Displaying messages 2 - 22 of 202 >>", then on clicking "<<", it shows "<< Displaying messages 0 - 20 of 202 >>" and after clicking ">>", we are on "<< Displaying messages 20 - 40 of 202 >>". As you can see, because of the above bug, it shows 0 :) Correct would be 1 -2 of 202 which is a bit silly, but the same happens with the default pager, just at the end of the thread.

The problematic part is this:

That still tries to check if $start is bigger than message_count, but instead, should check if $start is < 1. However, that conflicts with your existing $start < 0 check, which needs to be moved further down (and needs to become a < 1 check, since 0 - 1 would be invalid).

Yeah, I know, it's complicated and it wasn't easy to get right. If you can add any comments that make that code easier to understand, you're welcome to do so! I'm ok with delaying the 1.0 release until this is in.

If you can get it working, I'd heavily suggest that you install Simpletest, both things I've reported above are catched by the tests, to be exact, 8 test cases fail because of the first bug and two because of the second one. I can help you if you have any issues. Usually, it's a bit tricky first but once you're used to it, it is really worth it :) In short, if you have a good test coverage, you can freely hack away, run the tests (and fix your bugs) and can then be sure that your changes are good.

I'm on crack. Are you, too?

I think I have everything covered now - paging looks like such a simple task, but especially for messages, it is surprisingly involved.

I have added some comments, but the more the merrier - took me a while to get things understood.

I have also got rid of the border around the pager - that was not looking too good IMO.

I have just installed simpletest, but it seems to be frozen for me - How long is it supposed to take?

Attachment	Size
privatemsg.new_.patch	6.64 KB

and an alternative pager to consider - with this, the $max_amount is used for the paging, and only on the "real" last page (the one you view without a start=x) is the one that is shortened, so it acts more like a traditional pager and also required less special casing.

This approach also means that the visual positioning of messages stays the same, even if there are more replies to the thread.

You may want to compare the two patches to see which approach you prefer.

Attachment	Size
privatemsg.new_pager.patch	5.8 KB

and with another fix to the last patch

Attachment	Size
privatemsg.new_pager.patch	6.16 KB

Both have advantages and disadvantages. For example, with your second patch, "This approach also means that the visual positioning of messages stays the same, even if there are more replies to the thread." is only true as long as you don't switch to the next page, as you are left with just a single message. Imagine if you reply to a thread, and after reloading the page, you are left with just your response. Sure, on a second glance you'll see the changed pager but my first thought would be to assume that something went wrong.

Therefore, I'd like to go on with your patch in #50, and think about making it possible to choose between your two approaches in 2.x?

I'm currently looking at your patch in #50, there are still 7 fails and I'm currently figuring out why. Do you have time to join #drupal-games? We can also try to get your simpletest working.

All tests do now pass, changes:

- Replaced custom modulo calculation with % to avoid rounding bugs with floating point numbers
- Fixed handling of PRIVATEMSG_UNLIMITED
- Some other changes to make the code a bit simpler (I hope)

Attachment	Size
privatemsg_paging10.patch	6.95 KB

Commited the latest patch as discussed with nbz.

Unless there are new bugs reported, I will release privatemsg 6.x-1.0 during the next 24 hours!

Commited to 7.x-1.x-dev.