126793 - Templatise showdependencygraph.cgi

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Gervase Markham [:gerv]]

As above.

Gerv

Comment 1

[Andreas Franke (gone)]

Gerv, this won't be trivial because currently the html is generated before the
hide_resolved&maxdepth toolbar is displayed, but displayed afterwards.

Of course it would be nice to have everything templatized, but are you sure that
showdependency{tree,graph}.cgi are really _blockers_ for 2.16?

If you are doing it anyway, you could build an array (or tree) of hashes instead
of the html. Every hash could have information about a single line (= bug).

Comment 2

[Gervase Markham [:gerv]]

It is a blocker because it's user-facing. We should not be relaxing out criteria
for what we templatise any more than we have.

Take heart! All the templatisation bugs are now filed. There are no more hidden
surprises.

Gerv

Comment 3

[Gervase Markham [:gerv]]

Swapping titles due to misattachment of patch over in bug 126792..

Gerv

Comment 4

[Gervase Markham [:gerv]]

I've begun this, but can't go on until the local dot patch is checked in (bug
120537).

Gerv

Comment 5

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

The fix for bug 120537 has been checked in.  Don't delay, post the code today!

Comment 6

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Here's version 1. Let me know what you think :-) 

Gerv

Comment 7

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

Comment on attachment 77228 [details] [diff] [review]
Patch v.1

Index: showdependencygraph.cgi

>-graph [URL="${urlbase}query.cgi", rankdir=$::FORM{'rankdir'}, size="64,64"]
>-node [URL="${urlbase}show_bug.cgi?id=\\N", style=filled, color=lightgrey]

>+graph [URL="query.cgi", rankdir=$::FORM{'rankdir'}, size="64,64"]
>+node [URL="show_bug.cgi?id=\\N", style=filled, color=lightgrey]

I think URLBASE is required here because the URLs load from a link on 
the AT&T research site.

Comment 8

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Fixed Myk's comment.

Gerv

Comment 9

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

Comment on attachment 77479 [details] [diff] [review]
Patch v.2

Looks good, works. r=myk

Comment 10

[Bradley Baetz (:bbaetz)]

Comment on attachment 77479 [details] [diff] [review]
Patch v.2

>+      <td>
>+        Bug numbers:
>+        <input name="id" value="[% bug_id FILTER html %]">
>+      </td>

You don't need to html filter number + comma.

r=bbaetz apart from that, tested with a local dot binary.

Comment 11

[Dave Miller [:justdave]]

but it doesn't hurt anything, either, and it looks safer to a code auditor.

Comment 12

[Dave Miller [:justdave]]

-> patch author

(this is ready for checkin, btw)

Comment 13

[Bradley Baetz (:bbaetz)]

No, it looks like 'why are you filtering a number? Can this not be a number?'
etc, etc.

It won't make a difference, and it will be (very slightly) quicker not to filter.

Comment 14

[Gervase Markham [:gerv]]

Fixed.

Checking in showdependencygraph.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v  <-- 
showdependencygraph.cgi
new revision: 1.15; previous revision: 1.14
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/show/dependency-graph.html.tmpl,v
done
Checking in template/default/show/dependency-graph.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/show/dependency-graph.html.tmpl,v
 <--  dependency-graph.html.tmpl
initial revision: 1.1
done

Gerv