239346 - Need a hook after a bug's comments

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Nick Barnes]

User-Agent:       Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.6) Gecko/20040325
Build Identifier: 

The P4DTI Bugzilla integration with Perforce adds a section to a bug's display,
which appears after the comments.  In pre-templated versions of Bugzilla, this
was hard-coded in to bug_form.pl.  In 2.16.x, it was done with a custom
edit.html.tmpl template.  The right thing to do now is obviously to use a hook
which appears right after a bug's comments.  I have an "aftercomments" hook in
comments.html.tmpl which does just that.

Reproducible: Always
Steps to Reproduce:

Comment 1

[Nick Barnes]

Attachment: patch for comments.html.tmpl

Here is the simple patch to add the hook.

Comment 2

[Nick Barnes]

The URL link is to a screen shot of a Bugzilla bug in a P4DTI installation.  The
shot was taken with Bugzilla 2.12, but the intended behaviour hasn't changed
since then.

Comment 3

[Nick Barnes]

See also bug 207514 (Incorporate P4DTI patches into Bugzilla).

Comment 4

[Myk Melez [:myk] [@mykmelez]]

This looks fine.  The only question is whether you want the Perforce UI to
appear only on show_bug.cgi or also on long_list.cgi (i.e. when the user clicks
the "Long List" button at the bottom of a bug list).

If we put the hook in comments.html.tmpl, then unless the Perforce templates do
some additional tests for context, they'll get processed every time
comments.html.tmpl is included, which a quick grep of the source code says is
edit.html.tmpl (used by show_bug.cgi), show-multiple.html.tmpl (used by
long_list.cgi), and process/midair.html.tmpl (used by process_bug.cgi when two
users' changes conflict).

If you just want the Perforce UI in show_bug.cgi, then it would be better to
place the hook into edit.html.tmpl after the "PROCESS comments.html.tmpl" call
on line 536.  What do you think?

Comment 5

[Nick Barnes]

> This looks fine.  The only question is whether you want the Perforce UI to
> appear only on show_bug.cgi or also on long_list.cgi (i.e. when the user clicks
> the "Long List" button at the bottom of a bug list).

Yes, we want it for both.  In particular, we want it for "Format For Printing"
(which is long_list.cgi) as well as for show_bug.cgi.  We will need conditional
code to display the P4DTI section differently in these two cases, and to avoid
displaying it at all in the midair.html.tmpl case (when start_at > 0).  The
alternative for us would be to have two hooks, one where you suggest in
edit.html.tmpl and the other in show_multiple.html.tmpl.

Note that the current P4DTI patch does not show up in "Format For Printing" (in
2.16.x), but this is a bug in the patch.

Comment 6

[Gervase Markham [:gerv]]

Comment on attachment 145253 [details] [diff] [review]
patch for comments.html.tmpl

Nick: you probably want to avoid the process/midair.html.tmpl case, though...

Gerv

Comment 7

[Dave Miller [:justdave]]

a= justdave on condition that you're okay with it hitting in the mid-air template...

Comment 8

[Gervase Markham [:gerv]]

Dave: I'll add a comment just above the hook noting that anyone using it should
be aware that this template is called from multiple places, and they may need to
check in their hook template.

Hmm... ideally, the docs for hooks would tell you how to do that.

Gerv

Comment 9

[Gervase Markham [:gerv]]

Fixed.

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.10; previous revision: 1.9
done

Gerv

Comment 10

[Nick Barnes]

Great, thanks.

Comment 11

[Nick Barnes]

Attachment: Draft of P4DTI hook template

FYI, here is my draft of the actual hook file for the P4DTI.  You can see how
I'm using mode=='edit' and start_at == 0 to distinguish the various use cases.