644281 - When the sort order of a buglist is modified, the "Show next bug in my list" user pref still uses the original sort order to decide which bug to display next

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Kevin Hebert]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/534.13 (KHTML, like Gecko) RockMelt/0.9.48.58 Chrome/9.0.597.107 Safari/534.13
Build Identifier: 4.0

In my preferences, I have it the "After changing a bug" setting set to "Show next bug in list".  However, I noticed when changing bugs that it doesn't take the sort into account.  After changing a bug, I would get the bug that was next in the list before the sort.  I can't prove this as of yet, but I'm about 90% sure 3.6 took into account the sorted list.

Reproducible: Always

Steps to Reproduce:
1. Under Preferences, ensure that 'After changing a bug' is set to 'Show next bug in list.
2. Do a search in Bugzilla that results in a list (more than 3) bugs
3. Change the sort of the list by ID number (I always go reverse chronological)
4. Pick a bug, and make any change to it.
Actual Results:  
The result is that it shows the next bug in the pre-sorted list.

Expected Results:  
It should show the next bug in the post-sorted list.

This can be worked around by sorting your list, and then doing another search with the same criteria.  This displays the results with the method of sorting you previously used.  However, if you change the sorting again, it will break again.

Comment 1

[Max Kanat-Alexander]

Hey Kevin. Unfortunately, due to the nature of the "next bug" system, we'll need more information than this. We have to know the exact series of pages you went through to experience this problem (and what links you used to get from each page to each other page).

Comment 2

[Kevin Hebert]

Not a problem, Max.

query.cgi is the Search page I used.  I then did a search that made a list.  On the resulting page, I then sorted by ID.  Doesn't matter how many times you do it, or even what you sort by.  Then I clicked the top bug, which had the resulting URL: show_bug.cgi?id=4327.

The only links and pages I really went to were the Preferences page, Search page, the search results page, and then the bug pages.

Comment 3

[Max Kanat-Alexander]

Okay, interesting! We'll need somebody on our end to confirm this, and then we can look into it.

Comment 4

[:glob ✱]

confirmed on 4.0.

the buglist "next" and "prev" are also wrong after you change the sort order; they always use the original sort order to derive the next and previous bugs.

Comment 6

[Frédéric Buclin]

Marking as blocking 4.0.1 for now, but I will retarget it to 4.0.2 once this flag exists (unless someone can fix this bug before we release 4.0.1).

Comment 7

[Frédéric Buclin]

...per our current discussion at the IRC meeting...

Comment 8

[Frédéric Buclin]

Attachment: patch, v1

Comment 9

[Max Kanat-Alexander]

Hmm. I'd kind of rather fix this by having Search.pm sort bugs in the order received in bug_id if there's no other specified sort order, but that might not work, eh?

Comment 10

[Frédéric Buclin]

Well, if we store the sort order in the DB, then we have to update it every time we alter it, else I don't see the point to store it there.

Comment 11

[Max Kanat-Alexander]

  Oh, I had forgotten we were storing list_order. I wonder if we should stop? Or if it's valuable-enough to users to see the headers set with the proper sort order.

  So, looking over your code, I had a concern that newer searches could overwrite older ones if I used the same list_id in my URL. Is that true?

Comment 12

[Frédéric Buclin]

(In reply to comment #11)
>   Oh, I had forgotten we were storing list_order. I wonder if we should
> stop? Or if it's valuable-enough to users to see the headers set with the
> proper sort order.

Storing the sort order definitely makes sense. I wouldn't want one query to alter the sort order of other queries (depending on what I'm looking for, the sort order is not the same).


>   So, looking over your code, I had a concern that newer searches could
> overwrite older ones if I used the same list_id in my URL. Is that true?

That's the point, yes. When you update your query, e.g. by altering the sort order, you want to keep the same list ID, because it's the same query.

Comment 13

[Max Kanat-Alexander]

Okay. I'm concerned about some behavioral aspects of this patch (and possibly logical issues surrounding every possible solution) and so I want to do some more testing and thinking about it.

Comment 14

[Frédéric Buclin]

mkanat: could I get review for my patch? This bug is a blocker.

Comment 15

[Max Kanat-Alexander]

Comment on attachment 543798 [details] [diff] [review]
patch, v1

Review of attachment 543798 [details] [diff] [review]:
-----------------------------------------------------------------

All right, now that I've looked this over, I do actually like most all of it, in general. :-)

::: Bugzilla/User.pm
@@ +501,4 @@
>              }
>  
>              if ($search) {
> +                if (join(',', @{$search->bug_list}) ne join(',', @$bug_ids)) {

Okay, here's my concern. Now, when I go back and forward through my browser history, my search history will be updated in a way that is possibly confusing to me, if my browser re-runs the searches.

  Also, if I update my URL parameters directly (or, say, use an automated tool like BzAPI) I will continuously overwrite the same search.

  How about instead of this, we only update a search that has bugs if the list order has changed but the bug ids are otherwise identical? That doesn't solve the first issue, but it does help with the second issue, at least a bit.

  If there are new bug ids or missing bug ids, then we can create a whole new recent search entry.

Comment 16

[Frédéric Buclin]

Comment on attachment 543798 [details] [diff] [review]
patch, v1

I still pretend my fix is correct. Only updating the buglist when bugs are added or removed from it doesn't work. This means changing the sort order would be ignored, which we don't want (and is exactly what this bug is about). If the buglist ID is present, update this list if necessary. There is no reason to create another ID for it, else you could fill your history with the same buglist by editing the sort order.

Comment 18

[:glob ✱]

Comment on attachment 543798 [details] [diff] [review]
patch, v1

Review of attachment 543798 [details] [diff] [review]:
-----------------------------------------------------------------

> Okay, here's my concern. Now, when I go back and forward through my browser
> history, my search history will be updated in a way that is possibly confusing
> to me, if my browser re-runs the searches.

i've been unsuccessful in coaxing any browser into re-running the search when navigating back, so i don't think your concern is warranted.

r=glob

Comment 19

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified buglist.cgi
modified Bugzilla/User.pm
Committed revision 8041.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified buglist.cgi
modified Bugzilla/User.pm
Committed revision 7980.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified buglist.cgi
modified Bugzilla/User.pm
Committed revision 7668.

Comment 20

[Dave Hylands [:dhylands]]

I'm still seeing this problem.

For example, if I do the following search:
https://bugzilla.mozilla.org/buglist.cgi?list_id=4147708;emailtype1=exact;email1=dhylands%40mozilla.com;emailassigned_to1=1;query_format=advanced

and then click on the Status field, and click on the first bug in the list (which happens to be bug 744238 when I wrote this) it shows up as 3 of 18, rather than 1 of 18. Clicking Next takes me to 759169 (4 of 18) even though in the sorted list, the Next field would be 694594.

Comment 21

[Frédéric Buclin]

bmo uses a customized JavaScript code to handle column sorting which has nothing to do with the upstream code, see bug 425619. So the problem you are seeing is specific to bmo.