Bug 251656: Redesign dependency tree, part 1: fix the tree itself - Patch by André Batosti <batosti@async.com.br> r=myk r=LpSolit a=justdave

5 files changed, 359 insertions, 112 deletions

diff --git a/js/expanding-tree.js b/js/expanding-tree.js
new file mode 100644
index 000000000..d210a629c
--- /dev/null
+++ b/js/expanding-tree.js
@@ -0,0 +1,157 @@
+/* The contents of this file are subject to the Mozilla Public
+ * License Version 1.1 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.mozilla.org/MPL/
+ * 
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ * 
+ * The Original Code is the Bugzilla Bug Tracking System.
+ * 
+ * The Initial Developer of the Original Code is Netscape Communications
+ * Corporation. Portions created by Netscape are
+ * Copyright (C) 1998 Netscape Communications Corporation. All
+ * Rights Reserved.
+ * 
+ * Contributor(s): Mike Shaver <shaver@mozilla.org>
+ *                 Christian Reis <kiko@async.com.br>
+ *                 André Batosti <batosti@async.com.br> 
+ */
+
+if (!Node) {
+    // MSIE doesn't define Node, so provide a compatibility object
+    var Node = { TEXT_NODE: 3 }
+}
+
+if (!highlighted) {
+    var highlighted = 0;
+    var highlightedclass = "";
+    var highlightedover = 0;
+}
+
+function doToggle(node, event) {
+    var deep = event.altKey || event.ctrlKey;
+
+    if (node.nodeType == Node.TEXT_NODE)
+        node = node.parentNode;
+
+    var toggle = node.nextSibling;
+    while (toggle && toggle.tagName != "UL")
+        toggle = toggle.nextSibling;
+
+    if (toggle) {
+        if (deep) {
+            var direction = toggleDisplay(toggle, node);
+            changeChildren(toggle, direction);
+        } else {
+            toggleDisplay(toggle, node);
+        }
+    }
+    /* avoid problems with default actions on links (mozilla's
+     * ctrl/shift-click defaults, for instance */
+    event.preventBubble();
+    event.preventDefault();
+    return false;
+}
+
+function changeChildren(node, direction) {
+    var item = node.firstChild;
+    while (item) {
+        /* find the LI inside the UL I got */
+        while (item && item.tagName != "LI")
+            item = item.nextSibling;
+        if (!item)
+            return;
+
+        /* got it, now find the first A */
+        var child = item.firstChild;
+        while (child && child.tagName != "A")
+            child = child.nextSibling;
+        if (!child) {
+            return
+        }
+        var bullet = child;
+
+        /* and check if it has its own sublist */
+        var sublist = item.firstChild;
+        while (sublist && sublist.tagName != "UL")
+            sublist = sublist.nextSibling;
+        if (sublist) {
+            if (direction && isClosed(sublist)) {
+                openNode(sublist, bullet);
+            } else if (!direction && !isClosed(sublist)) {
+                closeNode(sublist, bullet);
+            }
+            changeChildren(sublist, direction)
+        }
+        item = item.nextSibling;
+    }
+}
+
+function openNode(node, bullet) {
+    node.style.display = "block";
+    bullet.className = "b b_open";
+}
+
+function closeNode(node, bullet) {
+    node.style.display = "none";
+    bullet.className = "b b_closed";
+}
+
+function isClosed(node) {
+    /* XXX we should in fact check our *computed* style, not the display
+     * attribute of the current node, which may be inherited and not
+     * set. However, this really only matters when changing the default
+     * appearance of the tree through a parent style. */
+    return node.style.display == "none";
+}
+
+function toggleDisplay(node, bullet) {
+    if (isClosed(node)) {
+        openNode(node, bullet);
+        return true;
+    }
+
+    closeNode(node, bullet);
+    return false;
+}
+
+function duplicated(element) {
+    var allsumm= document.getElementsByTagName("span");
+    if (highlighted) {
+        for (i = 0;i < allsumm.length; i++) {
+            if (allsumm.item(i).id == highlighted) {
+                allsumm.item(i).className = highlightedclass;
+            }
+        }
+        if (highlighted == element) {
+            highlighted = 0;
+            return;
+        }
+    } 
+    highlighted = element;
+    var elem = document.getElementById(element);
+    highlightedclass = elem.className;
+    for (var i = 0;i < allsumm.length; i++) {
+        if (allsumm.item(i).id == element) {
+            allsumm.item(i).className = "summ_h";
+        }
+    }
+}
+
+function duplicatedover(element) {
+    if (!highlighted) {
+        highlightedover = 1;
+        duplicated(element);
+    }
+}
+
+function duplicatedout(element) {
+    if (highlighted == element && highlightedover) {
+        highlightedover = 0;
+        duplicated(element);
+    }
+}
+
diff --git a/showdependencytree.cgi b/showdependencytree.cgi
index 03abb2729..efc26a2b2 100755
--- a/showdependencytree.cgi
+++ b/showdependencytree.cgi
@@ -65,7 +65,7 @@ if ($maxdepth !~ /^\d+$/) { $maxdepth = 0 };
 # have to embed a conditional statement into each query.
 my $milestone_column = Param('usetargetmilestone') ? "target_milestone" : "''";
 
-# The greatest depth to which either tree goes.
+# Stores the greatest depth to which either tree goes.
 my $realdepth = 0;
 
 # Generate the tree of bugs that this bug depends on and a list of IDs
@@ -148,12 +148,14 @@ sub GetBug {
     if (Bugzilla->user->can_see_bug($id)) {
         ($bug->{'exists'},
          $bug->{'status'},
+         $bug->{'resolution'},
          $bug->{'summary'},
          $bug->{'milestone'},
          $bug->{'assignee_id'},
          $bug->{'assignee_email'}) = $dbh->selectrow_array(
                 "SELECT 1,
                         bug_status, 
+                        resolution,
                         short_desc, 
                         $milestone_column, 
                         assignee.userid, 
diff --git a/skins/standard/dependency-tree.css b/skins/standard/dependency-tree.css
new file mode 100644
index 000000000..859ea199a
--- /dev/null
+++ b/skins/standard/dependency-tree.css
@@ -0,0 +1,90 @@
+/* The contents of this file are subject to the Mozilla Public
+  * License Version 1.1 (the "License"); you may not use this file
+  * except in compliance with the License. You may obtain a copy of
+  * the License at http://www.mozilla.org/MPL/
+  *
+  * Software distributed under the License is distributed on an "AS
+  * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+  * implied. See the License for the specific language governing
+  * rights and limitations under the License.
+  *
+  * The Original Code is the Bugzilla Bug Tracking System.
+  *
+  * The Initial Developer of the Original Code is Netscape
+  * Communications
+  * Corporation. Portions created by Netscape are
+  * Copyright (C) 1998 Netscape Communications Corporation. All
+  * Rights Reserved.
+  *
+  * Contributor(s): Christian Reis <kiko@async.com.br>
+  *                 André Batosti <batosti@async.com.br>
+  */
+
+ul.tree {
+    padding-left: 0em;
+    margin-left: 1em;
+    display: block;
+}
+
+ul.tree ul {
+    padding-top: 3px;
+    display: block;
+}
+
+ul.tree li { 
+    /* see http://www.kryogenix.org/code/browser/aqlists/ for idea */
+    padding-top: 3px; 
+    text-indent: -1.2em; 
+    padding-left: 0.5em; 
+    padding-bottom: 3px;
+    list-style-type: none;
+    background: url("dependency-tree/bug-item.png") no-repeat;
+}
+
+ul.tree li a.b {
+    padding-left: 30px; 
+    margin-right: -14px; 
+    text-decoration: none;
+}
+
+ul.tree li a.b_open {
+    background: url("dependency-tree/tree-open.png") center no-repeat;
+    cursor: pointer;
+}
+
+ul.tree li a.b_closed {
+    background: url("dependency-tree/tree-closed.png") center no-repeat;
+    cursor: pointer;
+}
+
+.summ_info {
+    /* change to inline if you would like to see the full bug details
+     * displayed in the list */
+    display: none; 
+    font-size: 75%;
+}
+
+.hint {
+    font-size: 90%;
+    margin: 0.2em;
+    padding: 0.1em;
+}
+
+.hint h3, .hint ul {
+    margin-top: 0.1em;
+    margin-bottom: 0.1em;
+}
+
+.summ A, .summ_deep A {
+    text-decoration: none;
+    color: darkblue;
+}
+
+.summ_deep {
+}
+
+.summ_h A {
+    background-color: #ffffaa;
+    color: #333;
+    text-decoration: underline bold;
+}
diff --git a/template/en/default/bug/dependency-tree.html.tmpl b/template/en/default/bug/dependency-tree.html.tmpl
index 8b5ecd59c..8fd0c18e6 100644
--- a/template/en/default/bug/dependency-tree.html.tmpl
+++ b/template/en/default/bug/dependency-tree.html.tmpl
@@ -19,6 +19,7 @@
   # Contributor(s): Tobias Burnus <burnus@net-b.de>
   #                 Ville Skyttä <ville.skytta@iki.fi>
   #                 Myk Melez <myk@mozilla.org>
+  #                 André Batosti <batosti@async.com.br>
   #%]
 
 [% PROCESS global/variables.none.tmpl %]
@@ -26,136 +27,135 @@
 [% filtered_desc = blocked_tree.$bugid.summary FILTER html %]
 [% PROCESS global/header.html.tmpl
    title = "Dependency tree for $terms.Bug $bugid"
-   h1    = "Dependency tree for <a href=\"show_bug.cgi?id=$bugid\">$terms.Bug $bugid</a>"
+   h1    = "Dependency tree for 
+            <a href=\"show_bug.cgi?id=$bugid\">$terms.Bug $bugid</a>"
+   javascript_urls = ["js/expanding-tree.js"]
+   style_urls = ["skins/standard/dependency-tree.css"]
    h2    = filtered_desc
 %]
 
 [% PROCESS depthControlToolbar %]
-
-[%# Display the tree of bugs that this bug depends on. %]
-<h3>
-  [% IF hide_resolved %]
-    Open [% terms.bugs %]
-  [% ELSE %]
-    [% terms.Bugs %]
-  [% END %] 
-  that <a href="show_bug.cgi?id=[% bugid %]">[% terms.bug %] [%+ bugid %]</a> 
-  depends on</h3>
-[% IF dependson_ids.size > 0 %]
-  (
-  [% IF maxdepth -%]Up to [% maxdepth %] level[% "s" IF maxdepth > 1 %] deep | [% END %]
-  <a href="buglist.cgi?bug_id=[% dependson_ids.join(",") %]">view as [% terms.bug %] list</a>
-  [% IF canedit && dependson_ids.size > 1 %]
-    | <a href="buglist.cgi?bug_id=[% dependson_ids.join(",") %]&amp;tweak=1">change several</a>
-  [% END %])
-  [% INCLUDE display_tree tree=dependson_tree bug_id=bugid %]
-[% ELSE %]
-  </h3>
-  <p>None</p>
-[% END %]
-
-[%# Display the tree of bugs that this bug blocks. %]
-<h3>
-  [% IF hide_resolved %]
-    Open [% terms.bugs %]
-  [% ELSE %]
-    [% terms.Bugs %]
-  [% END %] 
-  that <a href="show_bug.cgi?id=[% bugid %]">[% terms.bug %] [%+ bugid %]</a> 
-  blocks</h3>
-[% IF blocked_ids.size > 0 %]
-  (
-  [% IF maxdepth -%]Up to [% maxdepth %] level[% "s" IF maxdepth > 1 %] deep | [% END %]
-  <a href="buglist.cgi?bug_id=[% blocked_ids.join(",") %]">view as [% terms.bug %] list</a>
-  [% IF canedit && blocked_ids.size > 1 %]
-    | <a href="buglist.cgi?bug_id=[% blocked_ids.join(",") %]&amp;tweak=1">change several</a>
-  [% END %])
-  [% INCLUDE display_tree tree=blocked_tree bug_id=bugid %]
-[% ELSE %]
-  </h3>
-  <p>None</p>
-[% END %]
+  
+[% INCLUDE tree_section ids=dependson_ids type=1 %]
+ 
+[% INCLUDE tree_section ids=blocked_ids type=2 %] 
 
 [% PROCESS depthControlToolbar %]
 
+[% PROCESS global/footer.html.tmpl %]
+
 [%###########################################################################%]
-[%# Block to display a tree                                                 #%]
+[%# Tree-drawing blocks                                                     #%]
 [%###########################################################################%]
 
+[% BLOCK tree_section %]
+  [%# INTERFACE
+    #   - ids: a list of bug IDs to be displayed as children
+    #   - type: the type of tree. 1 = depends on, 2 = blockeds
+    # GLOBALS
+    #   - seen: Maintains a global hash of bugs that have been displayed
+    #%]
+  [% global.seen = {} %]
+  [%# Display the tree of bugs that this bug depends on. %]
+  <h3>
+    [% ids.size %] 
+    [% IF hide_resolved %]
+      Open [% terms.bugs %]
+    [% ELSE %]
+      [% terms.Bugs %]
+    [%- END %] 
+    that <a href="show_bug.cgi?id=[% bugid %]">[% terms.bug %] [%+ bugid %]</a> 
+    [% IF type == 1 %]
+        [% tree_name = "dependson_tree" %]
+        depends on
+    [% ELSIF type == 2 %]
+        [% tree_name = "blocked_tree" %]
+        blocks
+    [% END %]
+  </h3>
+  [% IF ids.size > 0 %]
+    ([% IF maxdepth -%]Up to [% maxdepth %] level[% "s" IF maxdepth > 1 %] deep | [% END -%]
+    <a href="buglist.cgi?bug_id=[% ids.join(",") %]">view as [% terms.bug %] list</a>
+    [% IF canedit && ids.size > 1 %]
+      | <a href="buglist.cgi?bug_id=[% ids.join(",") %]&amp;tweak=1">change several</a>
+    [% END %])
+    <ul class="tree">
+      [% INCLUDE display_tree tree=$tree_name %]
+    </ul>
+    <p>
+  [% ELSE %]
+    <p>None</p>
+  [% END %]
+[% END %]
+
+
 [% BLOCK display_tree %]
-[% tree.$bug_id.seen = 1 %]
-<ul>
-  [% FOREACH dep_id = tree.$bug_id.dependencies %]
-    [% dep = tree.$dep_id %]
-    <li>
-      [% "<script>document.write('<a href=\"#\" class=\"toggle\" onclick=\"listToggle(event); return false\">[-]</a>')</script>"
-         IF dep.dependencies.size > 0 && !dep.seen %]
-      [% isclosed = !dep.open %]
-      [% FILTER closed(isclosed) %]
-      <a href="show_bug.cgi?id=[% dep_id %]">[% dep_id %]
-        [[% IF dep.milestone %][% dep.milestone FILTER html %], [% END %]
-        [% dep.assignee_email FILTER html %]] -
-        [% IF dep.seen %]
-          <i>This [% terms.bug %] appears elsewhere in this tree.</i></a>
-        [% ELSE %]
-          [%+ dep.summary FILTER html %].</a>
+  [%# INTERFACE
+    #   - bugid: the ID of the bug being displayed
+    #   - tree: a hash of bug objects
+    #%]
+  [% bug = tree.$bugid %]
+  <li>
+    [%- INCLUDE bullet bugid=bugid bug=bug -%]
+    <span class="summ[% "_deep" IF bug.dependencies.size %]" 
+          id="[% bugid FILTER html %]" 
+          [% IF global.seen.$bugid %]
+            onMouseover="duplicatedover('[% bugid FILTER html %]')"
+            onMouseout="duplicatedout('[% bugid FILTER html %]')"
+          [% END %]>
+      [%- INCLUDE buglink bug=bug bugid=bugid %]
+    </span>
+    [% IF global.seen.$bugid %]
+      <b><a title="Already displayed above; click to locate"
+            onclick="duplicated('[% bugid FILTER html %]')"
+            href="#b[% bugid %]">(*)</a></b>
+    [% ELSIF bug.dependencies.size %]
+      <ul>
+        [% FOREACH depid = bug.dependencies %]
+          [% INCLUDE display_tree bugid=depid %]
         [% END %]
-      [% END %]
-      [% INCLUDE display_tree bug_id=dep_id
-           IF dep.dependencies.size > 0 && !dep.seen %]
-    </li>
+      </ul>
+    [% END %]
+  </li>
+  [% global.seen.$bugid = 1 %]
+[% END %]
+
+[% BLOCK bullet %]
+  [% IF bug.dependencies.size > 0 && ! global.seen.$bugid %]
+    [% extra_class = " b_open" %]
+    [% extra_args = 'onclick="return doToggle(this, event)"' %] 
   [% END %]
-</ul>
+  <a name="b[% bugid %]" 
+     class="b [%+ extra_class FILTER none %]"
+     title="Click to expand or contract this portion of the tree. Hold down the Ctrl key while clicking to expand or contract all subtrees."
+     [% extra_args FILTER none %]>&nbsp;&nbsp;</a>
 [% END %]
 
-<script type="text/javascript">
-if (typeof Node == 'undefined') {
-    /* MSIE doesn't define Node, so provide a compatibility object */
-    window.Node = {
-        TEXT_NODE: 3
-    };
-}
-
-function toggleDisplay(node)
-{
-    var display = node.style.display;
-    if (display == "none") {
-        node.style.display =
-            ("oldDisplay" in node) ? node.oldDisplay : "block";
-        return true;
-    }
-
-    node.oldDisplay = display;
-    node.style.display = "none";
-    return false;
-}
-
-function listToggle(event)
-{
-    var node = event.target;
-    if (!node)
-       node = event.srcElement;
-    if (node.nodeType == Node.TEXT_NODE)
-        node = node.parentNode;
-    var toggle = node.nextSibling;
-    while (toggle && toggle.tagName != "UL")
-        toggle = toggle.nextSibling;
-    if (toggle) {
-        node.firstChild.data = toggleDisplay(toggle) ? "[-]" : "[+]";
-    }
-}
-
-</script>
+[% BLOCK buglink %]
+  [% isclosed = !bug.open %]
+  [% FILTER closed(isclosed) -%]
+    <a title="[% INCLUDE buginfo bug=bug %]"
+       href="show_bug.cgi?id=[% bugid %]">
+      <b>[%- bugid %]:</b>
+      <span class="summ_text">[%+ bug.summary FILTER html %]</span>
+      <span class="summ_info">[[% INCLUDE buginfo %]]</span>
+    </a>
+  [% END %]
+[% END %]
 
-[% PROCESS global/footer.html.tmpl %]
+[% BLOCK buginfo %]
+  [% bug.status FILTER html %]
+  [%+ bug.resolution FILTER html %]; assigned to [% bug.assignee_email FILTER html %]; 
+  [% "Target: " _ bug.milestone IF bug.milestone %]
+[% END %]
 
 [%###########################################################################%]
 [%# Block for depth control toolbar                                         #%]
 [%###########################################################################%]
 
 [% BLOCK depthControlToolbar %]
- <table cellpadding="3" border="0" cellspacing="0" bgcolor="#d0d0d0">
- <tr>
+ <table cellpadding="3" border="0" cellspacing="0" bgcolor="#e0e0e0">
+   <tr>
    [%# Hide/show resolved button
        Swaps text depending on the state of hide_resolved %]
    <td align="center">
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index 57a323e7f..dbc618ee6 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -313,10 +313,8 @@
 'bug/dependency-tree.html.tmpl' => [
   'bugid', 
   'maxdepth', 
-  'dependson_ids.join(",")', 
-  'blocked_ids.join(",")', 
-  'dep_id', 
   'hide_resolved', 
+  'ids.join(",")',
   'maxdepth + 1', 
   'maxdepth > 0 && maxdepth <= realdepth ? maxdepth : ""',
   'maxdepth == 1 ? 1