This would make things a lot more consistant. I don't think it would be too much of a performance hit.

Blocks this bug the Bug 115650 too?

Yes, and there are lots of other places. I will try and submit a patch for this when 2.16 is out.

The User Interface component now belongs to Gerv. Reassigning all UNCONFIRMED and NEW (but not ASSIGNED) bugs currently owned by Myk (the previous component owner) to Gerv.

Reassigning back to Myk. That stuff about Gerv taking over the User Interface component turned out to be short-lived. Please pardon our confusion, and I'm very sorry about the spam.

Should GetBugLink become a HTML template, with the appropriate lookup information passed in? This way people could customise the links, and we wouldn't be passing in HTML into the template. Or has it already gone away?

Unloved bugs targetted for 2.18 but untouched since 9-15-2003 are being retargeted to 2.20 If you plan to act on one immediately, go ahead and pull it back to 2.18.

This bug has not been touched by its owner in over six months, even though it is targeted to 2.20, for which the freeze is 10 days away. Unsetting the target milestone, on the assumption that nobody is actually working on it or has any plans to soon. If you are the owner, and you plan to work on the bug, please give it a real target milestone. If you are the owner, and you do *not* plan to work on it, please reassign it to nobody@bugzilla.org or a .bugs component owner. If you are *anybody*, and you get this comment, and *you* plan to work on the bug, please reassign it to yourself if you have the ability.

I wasn't sure if buglist.cgi would also need this functionality since the other relevant information is already available. So I left it out.

Comment on attachment 208215 [details] [diff] [review] replaced most of the calls to GetBugLink() with bug->get_link >- # Old duplicate markers >- $text =~ s~(?<=^\*\*\*\ This\ bug\ has\ been\ marked\ as\ a\ duplicate\ of\ ) >- (\d+) >- (?=\ \*\*\*\Z) >- ~GetBugLink($1, $1) >- ~egmx; >- This needs to stay. Installations with dupes older than the check-in of bug 298273 want their legacy duping messages to continue to be linkified.

(In reply to comment #9) > (From update of attachment 208215 [details] [diff] [review] [edit]) > >- # Old duplicate markers > >- $text =~ s~(?<=^\*\*\*\ This\ bug\ has\ been\ marked\ as\ a\ duplicate\ of\ ) > >- (\d+) > >- (?=\ \*\*\*\Z) > >- ~GetBugLink($1, $1) > >- ~egmx; > >- > > This needs to stay. Installations with dupes older than the check-in of bug > 298273 want their legacy duping messages to continue to be linkified. > what's the problem? This will happen naturally.

(In reply to comment #10) > what's the problem? This will happen naturally. How? Are you linking numbers without "bug" in front of them?

(In reply to comment #11) > (In reply to comment #10) > > what's the problem? This will happen naturally. > > How? Are you linking numbers without "bug" in front of them? > Oops, no they are not.

Comment on attachment 208215 [details] [diff] [review] replaced most of the calls to GetBugLink() with bug->get_link Bitrotten. GetBugLink() has been moved to Template.pm. Moreover, links should be created using FILTER bug_link() instead.

*** Bug 329459 has been marked as a duplicate of this bug. ***

Paul, is this still your bug?

No, I'm not interested in it if it's about templates. I'm more interested in seeing it be part of the bug object.

*** Bug 93220 has been marked as a duplicate of this bug. ***

Can we start by at least using bug_link() in a buglist template?

Comment on attachment 596331 [details] [diff] [review] Use bug_link() in buglist.cgi There are probably much more places to fix than that. Don't upload one patch per template, please. :) Also, I don't think this template is the best one to start with. All the relevant data is already displayed in the buglist itself.

> There are probably much more places to fix than that. Don't upload one patch > per template, please. :) > There probably are, but those aren't the ones I'm concerned with right now and I didn't have time to fix them all. > Also, I don't think this template is the best one to start with. All the > relevant data is already displayed in the buglist itself. > Based on what I know and the feedback I've received, this is the best one to start with, and all relevant data is not necessarily displayed in the buglist itself. I guess I will stop submitting patches. What seems to make sense to me doesn't appear to be useful to the project and I don't have extra time to spend on things that are not going to be of use.

There a still calls to show_bug.cgi?id= for non-web templates and cases where bug_link doesn't support the attributes currently used, eg: list_id format ctype GoAheadAndLogIn class on the anchor tag Shall I open a new bug to add those features, or do that work here? Also, are we looking to replace mail/ical/rdf links to show_bug.cgi?id= ?

Comment on attachment 8539838 [details] [diff] [review] v3 Review of attachment 8539838 [details] [diff] [review]: ----------------------------------------------------------------- ::: template/en/default/bug/comments.html.tmpl @@ +201,4 @@ > [% END %] > > <span class="bz_comment_number"> > + [% comment_label FILTER bug_link(bug.bug_id, comment_num => comment.count) FILTER none %] this link should stay as is -- it's a pointer to the current bug, there's no need to take the overhead of running bug_link on every comment.

OK, I've switched that link to just use an anchor. What about the questions in comment 21?

(In reply to Matt Selsky [:selsky] from comment #21) > There a still calls to show_bug.cgi?id= for non-web templates i think it makes sense to restrict this work to web/html only, where the link can be augmented by the title. > and cases where bug_link doesn't support the attributes currently used adding support for the missing attributes makes a lot of sense; is it possible to update the bug/link template in a way that any unrecognised parameters are encoded into the url? to would avoid the need to explicitly iterate over all possible attributes.

Comment on attachment 8539886 [details] [diff] [review] v4 I disagree with most of your changes. You introduce huge performance penalties when the information is already available in the page. In many places, bug_link() has been intentionally avoided for this reason (see also bug 912531 for more information about this topic). >+++ b/template/en/default/account/prefs/email.html.tmpl >- <td><a href="[% urlbase FILTER html %]show_bug.cgi?id=[% bug.id FILTER uri %]"> >- [% bug.id FILTER html %]</a> >- </td> >+ <td>[% bug.id FILTER bug_link(bug.id) FILTER none %]</td> The bug status and bug summary are already displayed in the page. >+++ b/template/en/default/bug/comments.html.tmpl >- href="show_bug.cgi?id=[% bug.bug_id %]#c[% comment.count %]"> >- [%- comment_label FILTER html %]</a> >+ <a href="#c[% comment.count %]">[% comment_label FILTER html %]</a> This reverts bug 121112. If you do this, all links in "show multiple bugs at once" are broken again. >+++ b/template/en/default/bug/edit.html.tmpl >- <a href="show_bug.cgi?id=[% bug.bug_id %]"> >- [%-# %]<b>[% terms.Bug %]&nbsp;[% bug.bug_id FILTER html %]</b> >- [%-# %]</a> <span id="summary_container" class="bz_default_hidden"> >+ [% "$terms.Bug $bug.bug_id" FILTER bug_link(bug.bug_id) FILTER none %] I don't see the point to do this. In a bug report, the bug status and description are already displayed. >+++ b/template/en/default/bug/show-multiple.html.tmpl >- <a href="show_bug.cgi?id=[% bug.bug_id FILTER html %]">[% bug.bug_id FILTER html %]</a> >- [% IF bug.alias AND NOT bug.error %] ( >- [%- FOREACH alias IN bug.alias %]<a href="show_bug.cgi?id=[% alias FILTER uri %]"> >- [% alias FILTER html %]</a>[% UNLESS loop.last %], [% END %] >- [%- END %]) >+ [% "$terms.Bug $bug.bug_id" FILTER bug_link(bug.bug_id) FILTER none %] Same comment here. >+++ b/template/en/default/bug/show.html.tmpl >- The next [% terms.bug %] in your list is [% terms.bug %] >- <a href="show_bug.cgi?id=[% bug.bug_id %]">[% bug.bug_id %]</a>: >+ The next [% terms.bug %] in your list is >+ [%+ "$terms.bug $bug.bug_id" FILTER bug_link(bug.bug_id) FILTER none %]. Same comment here. >+++ b/template/en/default/list/table.html.tmpl >- <a href="show_bug.cgi?id=[% bug.bug_id %]">[% bug.bug_id %]</a> >+ [% bug.bug_id FILTER bug_link(bug.bug_id) FILTER none %] > [% ELSIF column == 'short_desc' || column == "short_short_desc" %] >- <a href="show_bug.cgi?id=[% bug.bug_id FILTER html %]"> >- [%- bug.$column.truncate(col_abbrev.maxlength, col_abbrev.ellipsis) FILTER html -%] >- </a> >+ [% bug.$column.truncate(col_abbrev.maxlength, col_abbrev.ellipsis) FILTER bug_link(bug.bug_id) FILTER none %] Those are huge performance regressions. If the user wants the bug status and summary, they can already be displayed as columns. For performance reasons, this bug is WONTFIX to me.

I think I'd want to see both a) a demonstration of a lack of performance impact and b) an explanation of why this makes the code significantly better before reviewing this patch. b) may well not be hard, but I don't see it in this bug yet. Gerv