259452 - Add bonsai style &mark support to showbug for bug comments

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[timeless]

I'd like to be able to markup a bug the way i can markup cvsblame/cvslog. The
idea is to highlight the important comments.

support should include simple comment number (0), ranges (2-3), and series of
ranges (5,7-9).

requirements for initial implementation:
- no user interface for generating such urls
+ css style coloring and table style coloring for older browsers

bonus points (can be done in second implementation):
+ an option to show/hide unmarked comments (cgi param)

bonus points (can be done in third implementation):
+ user interface to toggle showing/hiding unmarked comments

bonus points (can be done in fourth implementation):
+ user interface to dynamically mark/unmark comments (realtime for css capable
browsers) - this should alter the link at the top of the bug which is normally
just to the bug itself

Comment 1

[:glob ✱]

Attachment: initial attempt

note: doesn't do table style coloring for older browsers, as we're using css to
indicate other things (private messages, etc).

Comment 2

[timeless]

Attachment: follow on

our install (2.17.7) doesn't have the fancy header shading, so i'd like to be
able to color the header too. the css/global.css change here has the same
behavior as the one in glob's initial attempt, we'll have to use our own css
(dropping the pre portion of the rule) to get the behavior i forsee locally.

switching to defined instead of comparing against '' per justdave.
glob's version generated 
<pre
id="comment_text_1"class="bz_comment_hilite">thisisatestforwordwraptestscanyouverify</pre>


which isn't so great, my version is more careful about including spaces.

if people want to complain about the space before > they can, doing things this
way means you don't keep changing the line above you whenever you need to add
an attribute. i'm willing to stick the > on the preceding line, but want people
to consider cvsblame.

Comment 3

[Vlad Dascalu]

Comment on attachment 158960 [details] [diff] [review]
follow on

+    if (defined $cgi->param('mark')) {
+      foreach my $range (split ',', $cgi->param('mark')) {
+	 if ($range =~ /^(\d+)-(\d+)$/) {
+	   foreach my $i ($1..$2) {
+	     $marks{$i} = 1;
+	   }
+	 } elsif ($range =~ /^(\d+)$/) {
+	   $marks{$1} = 1;
+	 }
+      }
+    }

This is not commitable; the developers' guide suggest 4-space ident style, and
AFAIK 4-space ident style is already used in that file.

-<pre[% ' id="comment_text_' _ count _ '"' IF mode == "edit" %]>
+<pre[% ' id="comment_text_' _ count _ '"' IF mode == "edit" %]
+>

Nit: what's the need for this?

Comment 4

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

This looks useful.  The UI for this will take some thought, but that doesn't
need to be solved here.  I'm OK with this UI change.

My only nit is with the space between the background color for the header and
the background color for the comment, which looks messy and makes the header
seem disconnected from the comment.  The two background colors should butt up
against each other.  Perhaps the header and pre margins could be removed (and
padding added to the pre if necessary to make up for it)?


>if people want to complain about the space before > they can, doing things
>this way means you don't keep changing the line above you whenever you need
>to add an attribute. i'm willing to stick the > on the preceding line,
>but want people to consider cvsblame.

I see your point, but there's value in having the tag ending appear the way it
usually does and most people expect it to, even if it causes occasional cvsblame
pain.  I think it'll be better in the long run to stick the > on the preceding line.

Comment 5

[timeless]

Attachment: follow on with margin/padding changes

Comment 6

[Dave Miller [:justdave]]

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.

Comment 7

[Christian Reis]

Comment on attachment 159044 [details] [diff] [review]
follow on with margin/padding changes

Looks okay. I can't really see why we'd prefer to do this instead of
highlighting comments indicated by #c23 (and maybe it would be possible to
extend that by using #c23-41, but then again, maybe it wouldn't) but the
approver can decide on that. The code is sane.

Note that this clashes with private comment styling -- this may or may not be
acknowledged so I'm pointing it out.

Comment 8

[Dave Miller [:justdave]]

go for it

Comment 9

[timeless]

mozilla/webtools/bugzilla/show_bug.cgi 	1.30
mozilla/webtools/bugzilla/css/global.css 	1.6
mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl 	1.12