Closed Bug 49306 Opened 24 years ago Closed 24 years ago

Resolution field not set when mid-air collision happens

Categories

(Bugzilla :: Bugzilla-General, defect, P3)

x86
Windows 95

Tracking

()

RESOLVED FIXED
Bugzilla 2.12

People

(Reporter: leger, Assigned: Chris.Yeh)

References

Details

(Whiteboard: 2.12)

Attachments

(3 files)

I am starting to see several bugs with the resolution field empty. See: http://bugzilla.mozilla.org/show_bug.cgi?id=47358
i found 51 resolved bugs with a blank resolution select bug_id from bugs where bug_status="RESOLVED" and resolution=""; +--------+ | bug_id | +--------+ | 159 | | 176 | | 224 | | 225 | | 226 | | 227 | | 228 | | 256 | | 257 | | 267 | | 268 | | 279 | | 280 | | 281 | | 308 | | 331 | | 332 | | 348 | | 385 | | 388 | | 409 | | 413 | | 423 | | 523 | | 562 | | 610 | | 618 | | 622 | | 623 | | 630 | | 655 | | 657 | | 667 | | 683 | | 685 | | 7609 | | 8615 | | 14590 | | 15196 | | 29589 | | 28862 | | 33466 | | 33579 | | 35985 | | 36010 | | 36266 | | 42323 | | 45190 | | 47354 | | 47781 | | 49118 | +--------+ 51 rows in set (0.59 sec)
in bug 49118, hyatt closed the bug but no resolution was set. He was using 4.x and says this keeps happening. Dave, do you know how to reproduce it? What was the resolution supposed to be? It looks like invalid. I only see two bugs owned by hyatt with blank resolution and one of them was closed by someone else. Maybe he fixed them already. Looking at other bugs in that list i see what looks like people marking bugs fixed and 'fixed' not showing up. The last couple bugs might be worksforme or invalid.
*** Bug 31898 has been marked as a duplicate of this bug. ***
*** Bug 51668 has been marked as a duplicate of this bug. ***
Transferring 2.12 status from bug 51668. Also noting that this problem seems to be associated with midair collisions.
Whiteboard: 2.12
Can you go into more detail? How is it associated with mid-air collisions? In looking at the code, if a bug is marked resolved, it takes the form field for resolution and then changes the resolution to that. So somehow the bug resolution is getting set to 'empty' and some point and then 'resolved' sometime later using a null value for the resolution. How far back does the sqllog on www.mozilla.org go? It would be interesting to find all the sql calls for one of the bugs that ended up this way, and then try and figure out when the resolution got set to null.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Can you go into more detail? How is it associated with mid-air collisions? In looking at the code, if a bug is marked resolved, it takes the form field for resolution and then changes the resolution to that. So somehow the bug resolution is getting set to 'empty' and some point and then 'resolved' sometime later using a null value for the resolution. How far back does the sqllog on www.mozilla.org go? It would be interesting to find all the sql calls for one of the bugs that ended up this way, and then try and figure out when the resolution got set to null.
Status: RESOLVED → ASSIGNED
Resolution: FIXED → ---
I'm not 100% sure that it has anything to do with collisions but I did notice that it happened to a bug where I submitted my changes anyway on a 3-way collision.
Chris, a good bug to examine the sql log of is bug 50994, since this happened to it yesterday. It's now in the correct state though since timeless reopened and reresolved it.
I just duplicated this on a midair collision, too. I marked a bug RESOLVED/FIXED and got a midair collision. Since the other user had only submitted a comment, I picked to make my changes anyway. When I went back to look at the bug, the resolution field was blank.
I think this bug is actually a dupe of bug 61261.
Urp. I meant I think it's a dupe of bug 61529.
or vice versa. This one was here first. The comments on that one provide some good insight on where to look though.
*** Bug 61529 has been marked as a duplicate of this bug. ***
You can get a complete list of affected bugs with this link: http://bugzilla.mozilla.org/buglist.cgi?bug_status=RESOLVED&bug_status=VERIFIED&bug_status=CLOSED&resolution=--- Dan Mosedale commented in bug 61529 (dup of this one): | Notably the resolution of a bug is set outside the confines of the delta_ts | checks for mid-air collisions. So a bug can have its resolution successfully | changed, and then have other field changes (eg comments) fail with a mid-air | collision. Bug 61261 is an instance of this happening while a bug was being | DUPed. Terry, is there any reason not to move the resolution code inside | the collision checking guards? Updating summary.
Summary: Resolution field not displaying → Resolution field not set when mid-air collision happens
I am blind. I can't find the section of code that dmose is alluding to.
taking ownership
Assignee: tara → cyeh
Status: ASSIGNED → NEW
So I have a possible patch to fix this. Perhaps I'm missing something that makes this fix completely unworkable or causes some hideous side effect, but my initial testing looks promising. The problem was that there we delete the resolution field: delete $::FORM{'resolution'}; But if there was a midair collision, it would then try and determine what fields, if any had changed. Since the resolution form field had already been cleared, it thought that the user had meant to clear that from the bug. I fixed this by wrapping a check for mid-air collisions around the delete call and moving the call. Yes, it's messy and not pretty to have two mid-air collision checks, but this does stop the bug from happening on my setup and involves very little new code. If I could get a review and some testing by others, that would be great.
Status: NEW → ASSIGNED
Attached patch Another possible fix (deleted) — Splinter Review
After trying to figure out just why the heck the resolution field was being deleted to begin with, it appears it was because the CheckCanChangeField routine would choke on it. So I took a little different tack with it, and instead of moving the delete, just got rid of it, and fixed CheckCanChangeField to be able to deal with the resolution field. Does this make sense?
Attached patch Yet another (deleted) — Splinter Review
OK, here's another one, with a little different tack again. Still doing the cleanup within the CheckCanChangeField routine, but this one is a little cleaner, and will always wind up with the same result as the original code (it basically ignores the resolution field when checking permissions) but it no longer deletes the field.
This looks right from what I've seen of the code. Resolution will get added to the update a lot earlier which is why it was OK to delete it in the first place.
r=cyeh. the fix looks good. please feel free to check it in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
Checked in.
Sorry for the spam, but I needed to be able to query for all of these correctly.
Target Milestone: --- → Bugzilla 2.12
Moving closed bugs to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: