Closed
Bug 425668
Opened 17 years ago
Closed 16 years ago
Changing a bug (via process_bug) omits body classes and other header information when displaying the current/next bug
Categories
(Bugzilla :: User Interface, defect, P2)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.6
People
(Reporter: LpSolit, Assigned: mkanat)
References
Details
Attachments
(1 file, 3 obsolete files)
|
4.03 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
When you commit changes to a bug, the list of classes set to <body> are missing. You have to load the page again to see them (i.e. show_bug.cgi has them, but process_bug.cgi hasn't).
|
Assignee | |
Comment 1•17 years ago
|
||
This is another good reason why we need a special show_bug => 1 for header.html.tmpl.
|
|
||
Comment 2•17 years ago
|
||
This will get fixed with the other show_bug header flag fix.
|
|
Reporter | |
Comment 3•17 years ago
|
||
This bug is annoying for all those who have, like me or dveditz, added stuff to userContent.css to display security bugs differently. Everytime the bug is processed, it's being displayed as a normal bug. Should be trivial enough to fix.
Flags: blocking3.2?
|
|
Assignee | |
Comment 4•17 years ago
|
||
Not a blocker, but feel free to fix it and we can take it on all branches back to 3.0 if it's fixed before we release 3.2.
Severity: normal → minor
Flags: blocking3.2? → blocking3.2-
|
|
Reporter | |
Comment 5•17 years ago
|
||
The Bugzilla 3.0 branch is now locked to security bugs and dataloss fixes only. This bug doesn't fit into one of these two categories and is retargetted to 3.2 as part of a mass-change. To catch bugmails related to this mass-change, use lts081207 in your email client filter.
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
|
|
Assignee | |
Updated•16 years ago
|
Summary: Changing bugs omit body classes → Changing bugs omit body classes (centralize show_bug header stuff in templates)
Target Milestone: Bugzilla 3.2 → Bugzilla 3.6
|
|
Assignee | |
Updated•16 years ago
|
|
|
Assignee | |
Updated•16 years ago
|
Summary: Changing bugs omit body classes → Changing a bug (via process_bug) omits body classes and other header information when displaying the current/next bug
|
|
Assignee | |
Comment 12•16 years ago
|
||
This depends on the patch from the blocker.
|
|
Reporter | |
Comment 13•16 years ago
|
||
Comment on attachment 414145 [details] [diff] [review]
v1
We now get midair collisions all the time, and if you click "submit my changes anyway", then the last comment is missing.
Attachment #414145 -
Flags: review?(LpSolit) → review-
|
|
Assignee | |
Comment 14•16 years ago
|
||
Ah hah! The problem was caused by a NOW() in process_bug where there should have been a LOCALTIMESTAMP(0).
Attachment #414145 -
Attachment is obsolete: true
Attachment #416484 -
Flags: review?(LpSolit)
|
|
Reporter | |
Comment 15•16 years ago
|
||
Comment on attachment 416484 [details] [diff] [review]
v2
The midair collision problem is gone, but the comment you just added isn't displayed.
Attachment #416484 -
Flags: review?(LpSolit) → review-
|
|
Reporter | |
Updated•16 years ago
|
Whiteboard: [needs new patch asap]
|
|
Assignee | |
Comment 17•16 years ago
|
||
So, it turns out that the problem was that a Bugzilla::Bug doesn't update its internal structure well-enough during an update().
I tried, at first, modifying $bug->update() to delete all the stuff that it needed to delete, but it actually ended up being a lot of changes, and it seemed so fragile--there are so many interdependencies with a bug's values inside of the object itself (like how $self->user depends on so many things, for example) that it would be impossible to know what to delete, and we'd have to delete everything and re-load the bug from the database entirely, which seemed like a wasteful thing to do at the end of every single update (particularly because I would have put it in Bugzilla::Object, and that would have caused it to go everywhere in Bugzilla).
So instead I just create a new bug before displaying the template, which is much simpler.
Attachment #416484 -
Attachment is obsolete: true
Attachment #416787 -
Flags: review?(LpSolit)
|
|
Assignee | |
Updated•16 years ago
|
Whiteboard: [needs new patch asap]
|
|
Reporter | |
Updated•16 years ago
|
Attachment #416787 -
Flags: review?(LpSolit) → review-
|
|
Reporter | |
Comment 18•16 years ago
|
||
Comment on attachment 416787 [details] [diff] [review]
v3
wrong patch. It's the same as the previous one.
|
|
Assignee | |
Comment 19•16 years ago
|
||
Ah, thanks. :-) This is the updated patch.
Attachment #416787 -
Attachment is obsolete: true
Attachment #416856 -
Flags: review?(LpSolit)
|
|
Reporter | |
Comment 20•16 years ago
|
||
Comment on attachment 416856 [details] [diff] [review]
v4
>Index: process_bug.cgi
> # Include both action = 'same_bug' and 'nothing'.
> else {
>- $vars->{'bug'} = {bug_id => $cgi->param('id')};
>+ $vars->{'bug'} = $bug_objects[0];
> }
Nit: you could use $first_bug instead of $bug_objects[0] (same object, but as we have it...).
>+elsif ($action eq 'next_bug' or $action eq 'same_bug') {
Another nice cleanup would be to move lines below
# End the response page.
unless (Bugzilla->usage_mode == USAGE_MODE_EMAIL) {
into this block too as all other conditions do not trigger this code. This would make it more obvious when this code is called or not, for both developers and reviewers.
>Index: template/en/default/bug/process/header.html.tmpl
> [% PROCESS global/header.html.tmpl
> # We don't have a bug object at this point, unfortunately, so we can't
> # actually display all the proper header info.
>- header_addl_info = ''
> %]
The comment is no longer relevant. Please remove it on checkin.
Works fine. r=LpSolit
Attachment #416856 -
Flags: review?(LpSolit) → review+
|
|
Reporter | |
Updated•16 years ago
|
Flags: approval+
|
|
Assignee | |
Comment 21•16 years ago
|
||
(In reply to comment #20)
> Another nice cleanup would be to move lines below
>
> # End the response page.
> unless (Bugzilla->usage_mode == USAGE_MODE_EMAIL) {
>
> into this block too as all other conditions do not trigger this code. This
> would make it more obvious when this code is called or not, for both developers
> and reviewers.
Hmm. Unless I'm misunderstanding you, I think that that code is called at other times--sometimes the action is "nothing" and the same footer is printed whether there's a valid action or the action is "nothing".
I'll fix everything else on checkin, thanks for pointing them out. :-)
|
|
Assignee | |
Comment 22•16 years ago
|
||
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi
new revision: 1.426; previous revision: 1.425
done
Checking in template/en/default/bug/process/header.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/process/header.html.tmpl,v <-- header.html.tmpl
new revision: 1.16; previous revision: 1.15
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
|
|
Reporter | |
Comment 23•16 years ago
|
||
(In reply to comment #21)
> Hmm. Unless I'm misunderstanding you, I think that that code is called at
> other times--sometimes the action is "nothing" and the same footer is printed
> whether there's a valid action or the action is "nothing".
Oh, you are totally right. The last test is ($action ne 'nothing') but I read equals instead of not equals. So the code has to stay where it is now. Oops! :)
You need to log in
before you can comment on or make changes to this bug.
Description
•