103592 - truncate or wrap long email addresses in buglist.cgi

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Asa Dotzler [:asa]]

split off from bug 62729

There are several b.m.o users with email addresses in excess of 30 characters. In
cases where the reporter, qa contact or assignee are visible and of such great
length then all the other columns are shifted far to the right causing a
horizontal scroll and making bug number and summary not visible at the same
time. One solution to this problem would be to truncate or wrap email
addresses at some reasonable width. I can't ask these folks to get a new email
address but I wouldn't feel too bad asking them to abbreviate a long realname. 

For a testcase query b.m.o for bugs assigned to 
[e-mail]

Comment 1

[Christian Reis]

Sounds simple.

Comment 2

[Andreas Franke (gone)]

Possibly related:
Bug 61447: [patch] Column width control in bug lists

Comment 3

[Christian Reis]

Attachment: kiko_v1: implements fix with param "emailtruncatelength"

Comment 4

[Christian Reis]

I've added a parameter emailtruncatelength which is a solution to this bug, and
it's displayed at http://bugs.async.com.br/buglist.cgi

If people aren't happy with the Param() I'll change it, as long as we have
something to control it with. I think if bug 61447 lands all we would need is
the link part of this bug, but it's still much simpler and easier to review. Ah,
well.

Comment 5

[Gervase Markham [:gerv]]

I think we should just truncate at 30 characters and be done with it. We don't
need Yet Another Pref, because I can't think of anyone who would say "Actually,
I want my buglists to be gisburned - change that pref to 50 characters!".

We already truncate resolutions and summaries. Just truncate it at 30. It's a
two line patch. I'll review it.

Gerv

Comment 6

[Christian Reis]

Attachment: kiko_v2: fixes using magic number

Comment 7

[Christian Reis]

Well, gerv, I don't know. My CS teacher said "avoid magic numbers". I guess this
could change again when column widths kick in for good.

The rest of the (largish) patch is just tab -> space fixed (ugh).

Comment 8

[Gervase Markham [:gerv]]

Comment on attachment 52642 [details] [diff] [review]
kiko_v2: fixes using magic number

You seem to have some strange spacing issues here. Could you check that? 

The standard forms are:

} else {

and 

if (foo == bar)

Gerv

Comment 9

[Christian Reis]

Attachment: kiko_v3: fixes spacing issues

Comment 10

[Christian Reis]

Well, the spacing was already broken there when I hacked this in, but okay, I've
fixed it.

I'm a little bit concerned that the added visibility the links provide might not
be too good, but that's up to you.

Comment 11

[Christian Reis]

Attachment: kiko_v4: use SPAN TITLE instead, use maxemailsize variable.

Comment 12

[Gervase Markham [:gerv]]

Comment on attachment 52660 [details] [diff] [review]
kiko_v4: use SPAN TITLE instead, use maxemailsize variable.

r=gerv.

Gerv

Comment 13

[Christian Reis]

Attachment: kiko_v5: Changed tabs to spaces (write your congressman today: outlaw tabs)

Comment 14

[Gervase Markham [:gerv]]

Why is the param "9" instead of 30?

And why's this commented out:
+<!--<B>" .  time2str("%a %b %e %T %Z %Y", time()) . "</B>-->";

? :-)

Gerv

Comment 15

[Christian Reis]

Attachment: kiko_v6: reason: idiocy. I should go back to primary school and learn to do diffs

Comment 16

[Gervase Markham [:gerv]]

Comment on attachment 53130 [details] [diff] [review]
kiko_v6: reason: idiocy. I should go back to primary school and learn to do diffs

r=gerv

Gerv.

Comment 17

[Andreas Franke (gone)]

Comment on attachment 53130 [details] [diff] [review]
kiko_v6: reason: idiocy. I should go back to primary school and learn to do diffs

r=afranke

Comment 18

[Jacob Steenhagen]

Comment on attachment 53130 [details] [diff] [review]
kiko_v6: reason: idiocy. I should go back to primary school and learn to do diffs

Now they're all checked :)

>Index: buglist.cgi
...
>+                    $value = html_quote($value);

Anything that's gonna be put inside "" like $value is should be run through
value_quote() not html_quote() [html_quote() doesn't escape " marks]

Comment 19

[Christian Reis]

Attachment: kiko_v6_2: use value_quote (jesus)

Comment 20

[Jacob Steenhagen]

Comment on attachment 53133 [details] [diff] [review]
kiko_v6_2: use value_quote (jesus)

I'm going to assume that r=afranke and r=gerv still apply.
And for good measure, r=jake :)

Comment 21

[Gervase Markham [:gerv]]

Why do we have two functions? Why not one that does both? Escaping quotes in
HTML isn't harmful... Anyway. r=gerv again. Let's just check this in, OK? :-)

Gerv

Comment 22

[Jacob Steenhagen]

/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.145; previous revision: 1.144
done

Comment 23

[Andreas Franke (gone)]

Jake: What about the html_quote that's in the context of the diff? Is that one
wrong, too?

                 if ($::needquote{$c} || $::needquote{$c} == 5) {
                     $value = html_quote($value);
                 } else {

Comment 24

[Andreas Franke (gone)]

Oops, sorry, ignore my previous comment. I see what you mean.
Maybe this is a better question: Why don't the following code fragments use
html_quote instead of value_quote?

http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/buglist.cgi#1103

if ($::FORM{'debug'} && $serverpush) {
    print "<P><CODE>" . value_quote($query) . "</CODE><P>\n";
}

http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/buglist.cgi#788
if (defined $::FORM{'debug'}) {
    print "<P><CODE>" . value_quote($query) . "</CODE><P>\n";
}

Comment 25

[Christian Reis]

Andreas: as Gerv said, value_quote escapes " -> " and this isn't bad inside
HTML, so it's no big deal. html_quote would be simpler, but perhaps we might
want to migrate away from html_quote completely, as Gerv suggests.