304135 - Changing color in Dependency Graph

Status: NEW

(Bugzilla :: Dependency Views, enhancement)

Description

[Dmitry Pupkov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.2; ru-RU; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.2; ru-RU; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

It's a good idea about changing color from green in dependency graph.

Reproducible: Always

Steps to Reproduce:
You can select color with selector.
Actual Results:  
Green circles changes colors...

Comment 1

[Dmitry Pupkov]

Attachment: Patch about bug...

Comment 2

[Dmitry Pupkov]

Attachment: second file in patch

Comment 3

[Dmitry Pupkov]

This patch for Bugzilla version 2.18

Comment 4

[Stas Umansky]

Yeah, it saves printer catridges when you print in different colours

Comment 5

[Max Kanat-Alexander]

Hey Dmitry. Read our Contributor's Guide so that you know how to get your patch
accepted:

http://www.bugzilla.org/docs/contributor.html

Comment 6

[Dmitry Pupkov]

Attachment: Patch for adding color into Dependency Graphs

Comment 7

[Dmitry Pupkov]

Attachment: colorDependencyGraphPatch

Comment 8

[Jouni Heikniemi]

Comment on attachment 192208 [details] [diff] [review]
colorDependencyGraphPatch

Umm... I'm a bit confused. Do we need this one? Although I agree the current
green doesn't print well on B/W, but _why_ do we want to have this as user
configurable? Shouldn't this be a systemwide specification and thus set on the
code level? Or if it has to be user configurable, shouldn't it be a User
Preference?

In case we decide to go on with this (you might want to ask Justdave if he'll
accept this feature anyway), here are the review comments for the patch:


>Index: showdependencygraph.cgi
>===================================================================

>+my $graph_color = $cgi->param('graph_color') || "lightblue";

You're creating an DOT injection hole here (the user could paste any valid or
invalid DOT language into the graph_color param and get it included in the
dependency graph).


>Index: template/en/default/bug/dependency-graph.html.tmpl
>===================================================================
>-  Green circles represent open [% terms.bugs %].
>+  Color circles represent open [% terms.bugs %].

Nit: They're all "Color circles" (by definition). We need to come up with
something better.

>+			<option
>+            	[% " selected" IF graph_color == "green" %]>
>+				green
>+			</option>

You should include 'value="green"' style attributes to better support
localization (by example; it's not technically necessary here though).

Also, kill the tabs and run the testing suite to verify everything is OK with
the patch.

Comment 9

[Stas Umansky]

The main problem in hard-coded green colour is that it wastes the green printer
cartrige very fast when you print diagrams regularly. 
Besides, it is very useful to print diagram for different products / components
in different colour to tell one from another.