108987 - Linkify script to use quoteUrls on texts provided by user

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement, P2)

Description

[Stefan Seifert]

using kiko's description:
a page called bugify.cgi that offers a textarea, that works like this: enter
text with numbers interspersed and the numbers become links with title="summary"
added. this is for helping asa produce daily summaries and the weekly updates.

Comment 1

[Stefan Seifert]

Attachment: first version, would appreciate review

Comment 2

[GavinS]

Not a review, but I liked the idea, so some minor comments...

1) 'Name "main::usergroupset" used only once: possible typo at...' server log
warning

2) Should it (could it) run in taint mode?

3) (typo) apropriate -> appropriate

4) Captilize 'this', in 'this page lets...' maybe?

Comment 3

[Matthew Tuck [:CodeMachine]]

What does query/buglist fail to provide for this?

Comment 4

[Christian Reis]

MattyT, this is a generic textarea that swaps numbers for links with TITLEs set
-- it's of use to people that prepare textual summaries that include bug numbers
such as Asa's nightly build reports and the mozilla status report -- just
paste in a lot of text with numbers (you could paste in the full status report
if you wanted to) and get back the text with <A TITLE="" HREF="">666</A> in place
of the boring little numbers.

I'm sure this is of use to a lot of people (justdave said it "was cool")

Comment 5

[Matthew Tuck [:CodeMachine]]

OK, I'd assumed it was a link on the page as opposed to the link text.  Perhaps
this should be placed in a 'Bugzilla Utilities' section off the main page.  I'm
not sure what else we'd put there though, any suggestions?

Comment 6

[GavinS]

This is very similar to, if not a dupe of bug#101864, isn't it?

Comment 7

[Hampton Maxwell]

I've already written a full-blown script that does just this by querying
bugzilla with LWP.  It's what Asa is currently using to pre-process the build
bar comments.   You can access it at http://www.xsta.cc/bugreport.html with
source at http://www.xsta.cc/bugreport.cgi, feel free to add to the Bugzilla
utilities page.  Various keywords like bugzilla, #mozillazine,  etc. are
recognized and appropriate links inserted.  It will only handle 20 bug links at
a time as there seems to be a limit on bugzilla to prevent DoS attacks.

In bug #101864 (which is a dupe) it was suggested that this didn't belong in
Bugzilla, which is I implemented this separately.

Comment 8

[Gervase Markham [:gerv]]

http://www.xsta.cc/bugreport.cgi is 403 forbidden.

This definitely belongs in Bugzilla because it has wide utility - running the
weekly status reports through it would improve them immensely. 

Review comments:

It should be called linkify.cgi. :-)
It should not linkify bug numbers which do not exist. But maybe that's another bug. 
It should contain a full MPL Exhibit A, referencing you as the original author
of the code (assuming you didn't copy any from anywhere else.)

Gerv

Comment 9

[Hampton Maxwell]

> http://www.xsta.cc/bugreport.cgi is 403 forbidden.
make that http://www.xsta.cc/bugreport.pl

I will have to rewrite it to be a part of bugzilla, the current method doesn't
touch the database server at all.  I'll try and get that done over the weekend,
if someone wants to adapt my code to work under bugzilla before then, feel
free.I'll address your comments during the rewrite.

Comment 10

[Gervase Markham [:gerv]]

There is code in Bugzilla to do the quoting - you just need to call the
appropriate function. Please don't re-invent the wheel there. 

Any new CGIs checked into Bugzilla will need to be templatised. If you need help
with that, ask us. Check attachment.cgi in the current Bugzilla for examples of
how to do it.

Use an action= parameter to determine what the script should do; show the text
box by default.

Hope that helps :-)

Gerv

Comment 11

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 12

[Stefan Seifert]

Oh...just one week away and something like this happens.

Just a question: who should do this now? Would be no problem for me if Hampton
does this but I'm currently working on a new version which doesn't use quoteUrls
but GetBugLink cause the former likes to destroy the status update page if used
on the complete page.

Using templates is no problem, it's far better than the current prints ;)

Comment 13

[Hampton Maxwell]

Go ahead and take this.  I've got a lot on my hands right now and would need to
get more familiar with Bugzilla before I could do this.

Comment 14

[Stefan Seifert]

Okay new version ready: converted it to use the Template Toolkit. It now uses
action=raw instead of raw=1 and most important uses GetBugLink instead of
quoteUrls so you can process a complete weekly status update page :)

And it's called linkify as of Gerv's comment ;)

Bugs which don't exist are linked. To avoid this requires patching GetBugLink()

Comment 15

[Stefan Seifert]

Attachment: New version, adresses comments

cannot make attachment 56994 [details] obsolete, don't have sufficient rights.

Comment 16

[Stefan Seifert]

Attachment: Template for linkify.cgi script

place it in template/default/linkify/

Comment 17

[Jacob Steenhagen]

GetBugLink() was patched in bug 86300 to not link a bug if it doesn't exist
(this patch hasn't landed on b.m.o yet, so bug 1 is still a link until it does
land :)

Comment 18

[Gervase Markham [:gerv]]

Stefan: you now have the necessary permissions; but I don't have time to review
your patch :-(

Gerv

Comment 19

[Dave Miller [:justdave]]

*** Bug 101864 has been marked as a duplicate of this bug. ***

Comment 20

[Dave Miller [:justdave]]

Comment on attachment 60158 [details]
New version, adresses comments

The template calling system has changed since this patch was created, and we
also have moved the template directory into language-specific directories.  The
template code is glocal now so I think you just need to add stuff to $vars and
call $template->process now (search path should be set for you already)

Comment 21

[Dave Miller [:justdave]]

Comment on attachment 60159 [details]
Template for linkify.cgi script

Template looks good, but is Myk really the contributor?  This file was uploaded
by Stefan by the look of it...

Comment 22

[Gervase Markham [:gerv]]

The template is badly formatted.

The CGI:
- duplicates loads of stuff from e.g. globals.pl
- is dual-licensed, when the standard Bugzilla license is the MPL
- Uses GetBugLink, when quoteUrls would have been more appropriate

It's far simpler than this makes out. You could almost implement it using
page.cgi, if you made quoteUrls available to everyone (which is reasonable; it's
used in 3 CGIs already.)

Gerv

Comment 23

[Gervase Markham [:gerv]]

Attachment: Patch v.1

This is a "page.cgi"-based version. It's all templates apart from two tweaks,
which are reasonable generic things to do.

Gerv

Comment 24

[Bradley Baetz (:bbaetz)]

... and here you see the problem with adding this stuff into checksetup - new
installations won't get this.

Apart form taht, the comment on quoteUrls is wrong, and if you add it in
globally, you should pull it out of long_list and bug_form. (Of course, it
should be a FILTER, but thats another bug)

Comment 25

[Gervase Markham [:gerv]]

> ... and here you see the problem with adding this stuff into checksetup - new
> installations won't get this.

OK, well it's not too late to change the mapping mechanism. What do you suggest?

Another option is:
template/en/default/pages/<id>.tmpl.

and the ids acquire a content type, e.g.:
page.cgi?id=linkify.html

That has a certain neatness to it.

Gerv

Comment 26

[Bradley Baetz (:bbaetz)]

You could do that, or just not allow formats for page stuff. The main advantage
for formats is that you can share the same collected data, but there is no
collected data for pages....

How were you planning to get teh content type for pages anyway? Its currently
hardcoded to text/html.

Comment 27

[Gervase Markham [:gerv]]

There's a rather useful mapping in localconfig.

I'll open another bug for this.

Gerv

Comment 28

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Now this bug's dependencies are fixed, we can have this. 

It's basically a couple of files for page.cgi, plus a few tweaks, and some
fixes to page.cgi. For such a small file, I seem to be getting it wrong rather
a lot. Anyway.

Gerv

Comment 29

[Christian Reis]

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

>-if (defined $::FORM{'id'}) {
>+if ($::FORM{'id'}) {
>     $::FORM{'id'} =~ s/[^\w\-\.]//g;
>-    $::FORM{'id'} =~ /(.*)(\.(.*))?/;
>+    $::FORM{'id'} =~ /(.*)\.(.*)/;

This section deserves a comment. I *think* I know what it does
(removes anything non-alphanumeric, - and ., get two blocks
separated by dots) but I'm not sure why. Just an overall comment.

>+[% INCLUDE global/header.html.tmpl title = "Linkify Text" %]
>+
>+<p>
>+  If you enter some text, this form will return it with valid bug numbers, URLs
>+  and so on replaced with appropriate HTML links.
>+</p>

This text should explain that bug numbers should follow the format "bug 54332".
What does "and so on" mean? Maybe:

This form will add HTML links to bugs and URLs.

Hint: to linkify a bug, use the format <CODE>bug <I>bug_id<I></CODE>, as in 
<CODE>bug 3242</CODE> or <CODE>bug gerv_rocks<CODE>.

>+<form action="page.cgi" method="post">
>+  <textarea cols="100" rows="20" name="text"></textarea>
>+  <br>    
>+  <input type="hidden" name="id" value="linked.html">
>+  <input value="Linkify" type="submit">
>+</form>

We could get away with 80 columns and not make it horizontally-scroll on
640px displays. I would *really* like that change.

The patch looks and works fine, r=kiko. No need for 2r=.

Comment 30

[Gervase Markham [:gerv]]

Fixed. I looked carefully; this shouldn't conflict with groups.

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.202; previous revision: 1.201
done
Checking in page.cgi;
/cvsroot/mozilla/webtools/bugzilla/page.cgi,v  <--  page.cgi
new revision: 1.7; previous revision: 1.6
done
Checking in long_list.cgi;
/cvsroot/mozilla/webtools/bugzilla/long_list.cgi,v  <--  long_list.cgi
new revision: 1.30; previous revision: 1.29
done
Checking in bug_form.pl;
/cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v  <--  bug_form.pl
new revision: 1.103; previous revision: 1.102
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.145; previous revision: 1.144
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linked.html.tmpl,v
done
Checking in template/en/default/pages/linked.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linked.html.tmpl,v
 <--  linked.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linkify.html.tmpl,v
done
Checking in template/en/default/pages/linkify.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linkify.html.tmpl,v
 <--  linkify.html.tmpl
initial revision: 1.1
done

Gerv