User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0; .NET CLR 1.0.3705) Build Identifier: I would like to see: the following options forwards: report, 1, 2, 3, 4, MostRecentComment [current setup] R, MRC, 1, 2, 3, 4 MRC, R, 1, 2, 3, 4 reverse: MRC, 4, 3, 2, 1, R [my favorite] R, MRC, 4, 3, 2, 1 Reproducible: Always Steps to Reproduce:

Ooh, no. This would be horribly confusing. At the moment, you can: a) Rely on any view of Bugzilla, including the one on your mate's desk, to work the same b) Say "the comment above" in a comment and know that it's always right. c) Use Ctrl-End to get to the last comment written. All of these would break if this option was implemented. It would also be Yet More Configuration. And those sites who want it can achieve it using custom templates if they are masochists :-) Gerv

You make me laugh, Point taken, Then I would settle for REVERSE only. I want to see the last few updates at the begining, when posting a reply I hate scroll up, type, scroll down read, REPEAT until dizzy. Also my laptop has horribly slow (19.2 Max) cell connection, and pages which are very long bore me. It is fun to do QA in the Park :)

if not configurable, i would prefer REVERSE order, too. most of the time newer comments or attachments are more interesting than older ones!

How about making this a user definable option? It'll probably require some type of "commentflags" field in the profiles table.

Good idea. That depends on bug 98123, though, because we're shoving too much stuff in the profiles table.

use a cookie? maybe like the suggestion on bug 262592

I have a patch just about ready for this; I hope to get it in before the freeze.

I don't know if there's a separate ticket for this, but I think it belongs with this ticket. If we're going to put the effort to support reverse the comments on a per user basis, can there also be a flag to email *all* the comments whenever somebody makes a change? I myself hate having all the comments resent. But I do have co-workers who consider Bugzilla "inferior" because it doesn't include cited text from previous comments, similar to what you would get via email.

(In reply to comment #8) > I don't know if there's a separate ticket for this, but I think it belongs > with this ticket. No, it definitely doesn't. It's a fine idea deserving of consideration, but "No (preference) option to reverse sort the comments stack" does not have anything much to do with No (preference) option to mail me all comments from a bug every time". It's true that they both deal with comments, but it would be MORE accurate to say that one deals with how comments are displayed (show_bug.cgi) and the other deals with how comments are mailed (BugMail.pm). It's also true that they both deal with user preferences, but that's irrelevant now bug 98123 has landed and the infrastucture now exists to add user preferences for just about anything. Not trying to shoot down your idea, Albert; I think it's worthy of consideration. Please check that no dupe already exists, and if not then open up a new one and set it as being blocked by bug 98123. (That's how I'm keeping track of User Preferences, which I'm pretty much the one responsible for just now.) > If we're going to put the effort to support reverse the comments on a per user > basis, can there also be a flag to email *all* the comments whenever somebody > makes a change? > I myself hate having all the comments resent. But I do have co-workers who > consider Bugzilla "inferior" because it doesn't include cited text from previous > comments, similar to what you would get via email.

(In reply to comment #9) Thanks Shane. Found an existing ticket, bug #116863. Have added the dependency.

Of the methods mentioned in the initial description, only the following three are implemented by this patch: D= Description MRC = Most recent comment D, 1, 2, 3, 4, MRC as "oldest_to_newest" [this is the current setup] MRC, 4, 3, 2, 1, D as "newest_to_oldest" D, MRC, 4, 3, 2, 1 as "newest_to_oldest_desc_first" Just for the record... a version of this patch (without the modifications for 98123, so basically just a hack job) has been on our site for the last year at least; it is from that mod that I learned that I needed to fix up the midair collisions to *ignore* a user's preferences... something I missed the first time I tried to code it.

Fixed patch.

Probably a nit, but shouldn't BugMail.pm be updated as well? For cases where there is a delay in sending emails, I believe Bugzilla would email multiple comments.

Albert, are you on the right bug? I thought we agreed that this one was about how the comments looked to people when displayed by show_bug.cgi, and had nothing to do with BugMail at all?

> Albert, are you on the right bug? I thought we agreed that this one was about > how the comments looked to people when displayed by show_bug.cgi, and had > nothing to do with BugMail at all? Yup, I'm on the right bug... I think.... :) My point is that the comment stack ought to be consistent for both show_bug and BugMail.pm. Granted, you normally only get 1 comment from BugMail but if the sendmail flag is turned off, you could get multiple comments from a single email. I think those comments ought to be sorted the same way. This is not to be confused with making BugMail send all the comments. I'm aware that's a different issue marked in Bug #116863.

no, this bug is ONLY for the option, if a new bug [bug 116863?] is to be filed on the bugmail to make use of it then so beit.

(In reply to comment #16) > no, this bug is ONLY for the option, if a new bug [bug 116863?] is to be filed > on the bugmail to make use of it then so beit. Understood. I've added an initial patch to bug #116863. Needs more work, but it'll do.

Comment on attachment 177429 [details] [diff] [review] Code patch for tip, take 2 >- $vars->{'comments'} = Bugzilla::Bug::GetComments($id); >+ >+ # Always sort midair collision comments oldest to newest, >+ # regardless of the user's personal preference. >+ $vars->{'comments'} = Bugzilla::Bug::GetComments($id, "oldest_to_newest"); I'm not a huge fan of passing around fixed strings, but I suppose I'll live. >Index: Bugzilla/Bug.pm >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v >retrieving revision 1.67 >diff -u -r1.67 Bug.pm >--- Bugzilla/Bug.pm 10 Mar 2005 22:34:06 -0000 1.67 >+++ Bugzilla/Bug.pm 14 Mar 2005 22:32:02 -0000 >@@ -352,12 +352,27 @@ > } > > sub longdescs { >- my ($self) = @_; >+ my ($self, $comment_sort_order) = @_; > [snip] I'm fairly sure that this change and your comments.html.tmpl change are redundant. I belive that comments.html.tmpl is usually getting its data from this longdescs sub, here. And anywhere that isn't, ought to be. I don't understand why we have to change the sort and then reverse it again in the template. Also, just put these changes into GetComments, and change the actual sort order of the returned SQL data. Then it should be easy to get the desc and put it up front if needed, because it will be at the end of the returned data. It'd just be a unshift(@array, pop @array). Oh, wait... OK, now I kind of understand the comments.html.tmpl code. That seems like a lot of logic to have in a template... is there any way to simplify that logic? Also, why do we grant a special exte >+ [% IF is_description %] >+ <table> >+ <tr> >+ <td align="left"> >+ <b><a name="c0" href="#c0">Description</a>:</b>&nbsp;&nbsp;<script Couldn't we just add a description_at_end thing to the interface of comments.html.tmpl, and simplify this somewhat? And maybe also a has_description, although I'm not certain about that. >+ "newest_to_oldest_desc_first" => "Newest to Oldest, but keep Description at the top", That will really be too long for the <select> box. Maybe just "Newest To Oldest, Description on Top"? Even that's kinda long, but it's shorter, at least.

(In reply to comment #18) > I don't understand why we have to change the sort and then reverse it > again in the template. I don't. The code here is about what order the comments are in the array when they are passed to comments.html.tmpl. When they get there, the code in that section is ONLY about putting the right 'Comment #X' on the comment line. A parameter is created for this subroutine to allow code to override the user's preference, which is necessary for midair collisions. > Also, just put these changes into GetComments, and change the actual sort > order of the returned SQL data. Yeah, okay. I had a valid reason for not doing it that way at one time (namely the interaction of this userpref with another one, and the second one heavily modified the GetComments routine) but with the new Setting stuff most of the things I had to worry about are gone now. Done. > That seems like a lot of logic to have in a template... is there any way > to simplify that logic? Not really, but maybe a little. Look at this version and see if you like it any better. > Couldn't we just add a description_at_end thing to the interface of > comments.html.tmpl, and simplify this somewhat? And maybe also a > has_description, although I'm not certain about that. I have NO idea what you're talking about here; every bug 'has_description', and it isn't until we get to comments that we know the sort order and can tell if the description is at the top or the bottom. This code is taken directly from edit.html.tmpl (where it used to reside) and brought here unchanged. Prior to this patch, one could count on the Description always being the top comment, so it was fine to have the HTML for the Description line before the call to comments.html.tmpl. Now, Description could be either at the top or the bottom, and only comments.html.tmpl knows/cares which it is, which is why that stuff had to be moved. > >+ "newest_to_oldest_desc_first" => "Newest to Oldest, but keep Description at the top", > That will really be too long for the <select> box. Looks fine in mine. On what grounds are you saying 'too long'? (The 32-character restriction is only for the database...) > Maybe just "Newest To Oldest, Description on Top"? Doesn't shorten it that much, and obfuscates the meaning IMHO.

Comment on attachment 177759 [details] [diff] [review] Code patch for tip, take 3 This works properly, and it looks much better. :-) But for some reason I now only have one setting (this one) on my General Settings page, even though the Quip pref is clearly Enabled.

(In reply to comment #20) > But for some reason I now only have one setting (this one) on my General > Settings page, even though the Quip pref is clearly Enabled. See bug #286357.

Midaired with Albert(In reply to comment #20) > But for some reason I now only have one setting (this one) on my General > Settings page, even though the Quip pref is clearly Enabled. Midaired with Albert about to say the same thing; what you're seeing is a side- effect of the issue raised in bug 286357 and has nothing to do with this patch.

Comment on attachment 177759 [details] [diff] [review] Code patch for tip, take 3 Oh, OK. :-)

Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.387; previous revision: 1.386 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.249; previous revision: 1.248 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.73; previous revision: 1.72 done Checking in template/en/default/bug/comments.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl,v <-- comments.html.tmpl new revision: 1.17; previous revision: 1.16 done Checking in template/en/default/bug/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.58; previous revision: 1.57 done Checking in template/en/default/global/setting-descs.none.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/setting-descs.none.tmpl,v <-- setting-descs.none.tmpl new revision: 1.3; previous revision: 1.2 done

*** Bug 214315 has been marked as a duplicate of this bug. ***