Open
Bug 85600
Opened 23 years ago
Updated 4 years ago
Target Milestones misordered in Bugzilla query form (JavaScript ignores sortkey)
Categories
(Bugzilla :: Query/Bug List, defect, P3)
Tracking
()
ASSIGNED
People
(Reporter: bryner, Assigned: michael.j.tosh)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
LpSolit
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
LpSolit
:
review-
|
Details | Diff | Splinter Review |
The target milestones on the query page are in a semi-random order after M16.
dmose says this is because of the collation keys not matching up between
products.
Comment 1•23 years ago
|
||
I've looked in Mozilla, Navigator 4.x and IE6 so this isn't a Mozilla bug (they
all display the list out of order). I've done all kinds of playing with the sort
order keys and nothing makes this any better. If you select just the Browser
Product in query.cgi, the list is all messed up so it's not likely a conflict
with sortkeys from other Products. I think this must be a Bugzilla bug. I did a
couple quick queries and didn't see it reported there so I'm sending this bug over.
Assignee: endico → justdave
Component: Bugzilla: Keyword & Component Changes → Bugzilla
Product: mozilla.org → Webtools
QA Contact: lchiang → matty
Version: other → Bugzilla 2.13
Comment 2•23 years ago
|
||
Just for comparison, can you give me a list of the milestones and sort keys for
the Browser product?
Comment 3•23 years ago
|
||
I was playing around and noticed that when I reset my query to the default with
http://bugzilla.mozilla.org/query.cgi?nukedefaultquery=1 that all my Target
Milestones seemed to line up just fine. Is is possible that by having only
several of Products selected in my default that it could have messed up the sort
order? It's working fine for me now.
Comment 4•23 years ago
|
||
...or maybe it was fixed by something we picked up in our sort of 2.14 update a
few days ago.
Comment 5•23 years ago
|
||
Hmm, looks like 2.14 or something done at the same time fixed it. Would be nice
to know what was wrong before resolving though.
Comment 6•23 years ago
|
||
Moving to new Bugzilla product ...
Assignee: justdave → endico
Component: Bugzilla → Query/Bug List
Product: Webtools → Bugzilla
Version: Bugzilla 2.13 → 2.13
Comment 7•23 years ago
|
||
I saw this come back the other day ... but hopefully with the landing of bug
#96534 this will be fixed due to the JS code being rewritten, except for bug
#97736.
Comment 8•23 years ago
|
||
OK, here's the deal.
Currently our JS does sorting without regard to sortkey (bug #97736). The old
code did sorting of sortkeys properly, but it may have done it within products
when you've selected a project.
But when you load query.cgi with no products selected it loads from the
versioncache rather than constructing a list in JS, and this leads to misordered
milestones, because it loads from the versioncache which sorts on sortkey rather
than sortkey within product.
So either:
- we should sort versioncache on just sortkey, mozilla.org needs to fix their
sortkeys to segregate the milestones, and the query JS needs to sort purely on
sortkey
- we should sort versioncache on just sortkey within products, and the query JS
needs to do the same
Updated•23 years ago
|
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.16
Comment 9•23 years ago
|
||
We are currently trying to wrap up Bugzilla 2.16. We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut. Thus this is being retargetted at 2.18. If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Comment 10•22 years ago
|
||
*** Bug 143660 has been marked as a duplicate of this bug. ***
Updated•21 years ago
|
Assignee: endico → nobody
Comment 11•21 years ago
|
||
nonody@bugzilla.org seems to be working on getting this into 2.18, so we'll
shoot for 2.20
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Comment 12•20 years ago
|
||
Yeah, this still happens on the trunk, too.
Summary: Target Milestones misordered in Bugzilla query form → Target Milestones misordered in Bugzilla query form (JavaScript ignores sortkey)
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Comment 13•19 years ago
|
||
*** Bug 97736 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Assignee: nobody → mkanat
Severity: normal → minor
Priority: P2 → P3
Comment 14•19 years ago
|
||
*** Bug 314504 has been marked as a duplicate of this bug. ***
Comment 15•19 years ago
|
||
It comes from the server already sorted. Why JavaScript at all?
<label for="target_milestone" accesskey="t">
<select name="target_milestone" id="target_milestone">
<option value="Bugzilla old">Bugzilla old
</option>
<option value="Bugzilla 2.12">Bugzilla 2.12
</option>
<option value="Bugzilla 2.14">Bugzilla 2.14
</option>
<option value="Bugzilla 2.16">Bugzilla 2.16
</option>
<option value="Bugzilla 2.18">Bugzilla 2.18
</option>
<option value="---">---
</option>
<option value="Bugzilla 2.20">Bugzilla 2.20
</option>
<option value="Bugzilla 2.22" selected>Bugzilla 2.22
</option>
<option value="Bugzilla 2.24">Bugzilla 2.24
</option>
<option value="Bugzilla 3.0">Bugzilla 3.0
</option>
<option value="Bugzilla 3.2">Bugzilla 3.2
</option>
<option value="Future">Future
</option>
</select>
</label>
Comment 16•19 years ago
|
||
Sorry, now I see the problem is that we need to merge milestones for a possible cross product search.... Hmmm.
Comment 17•19 years ago
|
||
I was looking into making a patch for this, but there are 4 different ways that are not so easily handled.
1. If one product is selected
(easy! show that product's milestones in proper order)
2. If two products are selected
(How do we mix the milestones? P1's first, then P2's? What if there are duplicate milestone names?)
3. If nothing is selected, is a milestone relevant? Won't we choose a product when we choose a milestone? Of course unless it's a duplicate)
4. On initial page load (do the same as #3, I would guess)
Should we just use case #1, and in all other cases just alphabetize?
Comment 18•19 years ago
|
||
(In reply to comment #17)
They should always be ordered first by sortkey and then alphabetically, in every case.
Comment 19•19 years ago
|
||
(In reply to comment #18)
> (In reply to comment #17)
> They should always be ordered first by sortkey and then alphabetically, in
> every case.
>
So if we have 2 products R1 & R2.
R1 has milestones AAA = 1, BBB = 2, CCC = 3
R2 has milestones ZZZ = 1, XXX = 2, YYY = 3
You want to see
AAA
ZZZ
BBB
XXX
CCC
YYY
is that right?
Comment 20•19 years ago
|
||
(In reply to comment #19)
> is that right?
Exactly! :-)
Assignee: mkanat → pdemarco
Updated•18 years ago
|
QA Contact: mattyt-bugzilla → default-qa
Updated•18 years ago
|
Group: webtools-security
Target Milestone: Bugzilla 3.0 → Bugzilla 2.24
Comment 21•18 years ago
|
||
Looks like Firefox did some strange selecting of the group radio boxes on the mass-change page when I clicked refresh in my browser before that last mass-change. This is not really a security bug. :-)
Group: webtools-security
Comment 22•18 years ago
|
||
This bug is retargetted to Bugzilla 3.2 for one of the following reasons:
- it has no assignee (except the default one)
- we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1)
- it's not a blocker
If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0.
If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug.
If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
Comment 23•18 years ago
|
||
*** Bug 360321 has been marked as a duplicate of this bug. ***
Comment 24•17 years ago
|
||
I'm not sure of the procedure to make a patch, but I've fixed this bug in BZ 3.0.
I made the fix in query.cgi. Instead of saving a "1" in the hash used to remove duplicates, I changed it to save the sortkey value. Then I sorted the milestones hash to build a sorted milestones array.
It works as before. The one issue with this fix is if there are duplicate milestones with different sortkeys, the key for the last milestone encountered is used. I keep my milestones pretty well synced so this isn't a problem for me but it may affect some other installations.
Details for the fix in query.cgi:
Change Line 194 to read:
$milestones{$_->name} = $_->sortkey foreach (@{$product->milestones});
Replace Line 199 with:
my @milestones = ();
foreach my $m (sort { $milestones{$a} <=> $milestones{$b} } keys %milestones)
{
push @milestones, $m;
}
I'm sure there's probably a way to get the second chunk all into one line, but I was just happy to have it fixed. :)
-Scott
Comment 25•17 years ago
|
||
Comment 26•17 years ago
|
||
Hey Brian! Thanks for the fix! :-) I think the Contributor's Guide explains how to make a patch:
http://www.bugzilla.org/docs/contributor.html
And if it doesn't, then you should come into IRC and we'll help you out. Details on how to get into IRC are here:
http://wiki.mozilla.org/Bugzilla:Communicate
Assignee: query-and-buglist → bryner
Comment 27•17 years ago
|
||
Attachment #265961 -
Attachment is obsolete: true
Attachment #265993 -
Flags: review?
Comment 28•17 years ago
|
||
Patch checked in and older attachment of query.cgi has been obsoleted.
This has been tested on my BZ 3.0 server.
-Scott
Updated•17 years ago
|
Attachment #265993 -
Flags: review? → review?(bugreport)
Comment 29•17 years ago
|
||
Comment on attachment 265993 [details] [diff] [review]
Patch of above fix to query.cgi
>+my @milestones = ();
>+foreach my $m (sort { $milestones{$a} <=> $milestones{$b} } keys %milestones)
>+{
>+ push @milestones, $m;
>+}
Why not writing:
my @milestones = sort { $milestones{$a} <=> $milestones{$b} } keys %milestones;
It's cleaner.
Comment 30•17 years ago
|
||
Even better. Like I said, I was sure there was a tighter way.
Comment 31•17 years ago
|
||
Comment on attachment 265993 [details] [diff] [review]
Patch of above fix to query.cgi
per my previous comment. Use the shorter form.
Attachment #265993 -
Flags: review?(bugreport) → review-
Comment 32•17 years ago
|
||
Your suggested change does not work. I actually tried it before I submitted the longer form.
Just re-tried your suggestion as-written and have confirmed that it does not work. The search page never completes loading.
Resetting Review flag on original working patch for reconsideration.
Comment 33•17 years ago
|
||
Comment on attachment 265993 [details] [diff] [review]
Patch of above fix to query.cgi
Suggested change by reviewer (and the reason for denial) does not work.
Attachment #265993 -
Flags: review- → review?(bugreport)
Comment 34•17 years ago
|
||
Comment on attachment 265993 [details] [diff] [review]
Patch of above fix to query.cgi
My suggestion works perfectly; I tested it.
Attachment #265993 -
Flags: review?(bugreport) → review-
Comment 35•17 years ago
|
||
Not on my system. Hey... I'm not going to argue about this. The patch I submitted works fine on my system. The line you posted, copied exactly, does not. If you REALLY decide you want it in the shorter form, then YOU fix it and submit the patch.
I was trying to be a good guy and share the fix of an annoying bug with the Bugzilla community at-large. But I'm not interested in playing little power games for 3 lines of code.
My fix works for me. Yours doesn't. Take which ever one you like. Or take neither. It's no skin off my back. MY SYSTEM is working properly. Either way, I'm done with this bug.
Assignee | ||
Comment 37•17 years ago
|
||
Seems like some bad comments here between people, I'm uploading the patch as requested by LpSolit to use the single line code. This does work for me and loads quickly.
If someone can PLEASE PLEASE PLEASE review, approve, and check in this code, I'd be appreciative.
Attachment #291476 -
Flags: review?(bugreport)
Updated•17 years ago
|
Attachment #291476 -
Flags: review?(bugreport) → review?(justdave)
Comment 38•17 years ago
|
||
Comment on attachment 291476 [details] [diff] [review]
Patch for query.cgi on 3.0.2
>-my @milestones = sort(keys %milestones);
>+my @milestones = sort { $milestones{$a} <=> $milestones{$b} } keys %milestones;
Nit: shouldn't you also sort them alphabetically when the sortkey is identical? Something like:
sort { $milestones{$a} <=> $milestones{$b} || $a cmp $b }
Assignee | ||
Comment 39•17 years ago
|
||
Ok, I agree, here is the patch.
Attachment #291476 -
Attachment is obsolete: true
Attachment #291476 -
Flags: review?(justdave)
Attachment #291663 -
Flags: review?(LpSolit)
Assignee | ||
Comment 40•17 years ago
|
||
or wouldn't the entries come in alphabetical order anyway? we don't need to resort them if they are the same. Sounds redundant.
Comment 41•17 years ago
|
||
(In reply to comment #40)
> or wouldn't the entries come in alphabetical order anyway? we don't need to
> resort them if they are the same. Sounds redundant.
They are not sorted alphabetically as they come from a hash.
Assignee: bryner → michael.j.tosh
Comment 42•17 years ago
|
||
This also seems to be an issue with the report.cgi. I'm not perl programmer so I wouldn't know where to start looking but I am getting strange results when using version like milestones even with or without sortkeys defined.
My report results are being sorted as follows even when using the sortkeys shown.
Milestone Sortkey
2.0.10 release 2010
2.0.11 release 2011
2.0.9 release 2009
Assignee | ||
Comment 43•17 years ago
|
||
Funny thing, I just found that today myself. Tabular reports don't sort the milestones correctly.
Comment 44•17 years ago
|
||
And just as a note, the patch (attachment#291663 [details] [diff] [review]) doesn't seem to correct this issue at least in my tests.
Assignee | ||
Comment 45•17 years ago
|
||
Sorting Milestones in report.cgi would be a new issue. you mean the tabular report, right? When you do a search it displays the table alphabetically.
For me, the query.cgi displays the sort correctly, even when editing or creating the query values.
Comment 46•17 years ago
|
||
Comment 47•17 years ago
|
||
Something is wrong with this patch. When you load query.cgi, the list is sorted based on the sortkey, as passed from query.cgi itself. But when you select a product and then unselect all products, you get the old behavior where all milestones are sorted alphabetically. It looks like you also have to fix the JS script to use milestones's sortkey instead of their name. Michael, could you investigate?
Status: NEW → ASSIGNED
Comment 48•17 years ago
|
||
Comment on attachment 291663 [details] [diff] [review]
UPDATED Patch for query.cgi on 3.0.2
r- for further investigation.
Attachment #291663 -
Flags: review?(LpSolit) → review-
Comment 49•17 years ago
|
||
I was going to submit a new bug, but this seems related enough.
I just upgraded from bugzilla 2.1.8 to 3.0.3, and noticed strange behavior with the target milestone list on the query page:
Initially, with no product selected, it is sorted alphabetically.
Once you choose a product, the milestones for that product are sorted by sortkey.
Now, the fun part...
De-select the product (using CTRL), and the milestone list now has multiple copies of several milestones. It should be able to filter out duplicate milestones.
(I should mention that we create the same milestones for multiple products, so that is probably a necessary prerequisite to seeing this bug)
I see above people are discussing version 3.2.x already... I apologize if this is something fixed long ago, but it seemed that 3.0.3 was recommended as the latest stable release.
Comment 50•15 years ago
|
||
Bugzilla 3.2 is restricted to security bugs only. Mass-retargetting to 3.6.
Target Milestone: Bugzilla 3.2 → Bugzilla 3.6
Comment 51•14 years ago
|
||
I upgrade from version 2.22 to 3.6. In 2.22 I found a workaround for me by editing globals.pl:253:
# reading target milestones in from the database - matthew@zeroknowledge.com
SendSQL("SELECT milestones.value, products.name " .
"FROM milestones, products " .
"WHERE products.id = milestones.product_id " .
"ORDER BY milestones.value DESC");
globals.pl doesn't exist any more in version 3.6. Is there any workaround to fix the unsorted list? Problem still exists by selecting only a classification (no product).
Comment 52•14 years ago
|
||
Bugzilla 3.6 is now restricted to security fixes only. We will retarget this bug once it has a patch ready for checkin.
Target Milestone: Bugzilla 3.6 → ---
Comment 53•4 years ago
|
||
Many years later and this still seems to be an issue. Anyone giving this some love? :)
You need to log in
before you can comment on or make changes to this bug.
Description
•