Closed Bug 80183 Opened 24 years ago Closed 23 years ago

configurable index page (using Template Toolkit)

Categories

(Bugzilla :: Bugzilla-General, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: jones, Assigned: jacob)

References

Details

Attachments

(9 obsolete files)

This is an enhancement request (with patch providing the functionality) to add a new 'index.cgi' script that can replace or supplement the existing index.html static html file. By having the main index page be a cgi program rather than static html, we can take advantage of the configuration params (like bannerhtml) which makes it far easier for sites to customize the HTML associated with their deployed version of Bugzilla. I will attach to this bug two files: a new "index.cgi" file, and a patch to the defparams.pl file that adds a new "index-template" param that contains the html to be displayed on the index page. In addition, the attached index.cgi file can (optionally) display a table of the products that are part of this Bugzilla instance. If that table is shown, the product names can optionally be links to a query for all of the bugs for that product. These optional features are turned off by default in order to emulate the original behavior of Bugzilla as closely as possible. Although this is not even close to a solution for separating UI from application logic, it does help a bit by making it easier to upgrade Bugzilla without having to reimplement HTML changes in the index.html file. A solution more along the lines of Bug 49225 in which XML is converted to HTML using a user-specified set of XSLT stylesheets would be preferable but more disruptive to implement. See bug 2900 and bug 51100 for related issues.
Attached patch Patch with new params for defparams.pl (obsolete) (deleted) — Splinter Review
Just for comparison when we're reviewing this... See also: Bug 69807: Make "My Bugs" the default starting page. Bug 51100: configurable "home page" per engineer and per team Bug 2900: PATCH to synchronize static html file with 'bannerhtml' param I'm not arguing against an index.cgi... I'm all for it in fact. I think it's been generally accepted that we need to go that route. There has always been a lot of arguments as to how to do it (see the above bugs).
Target Milestone: --- → Bugzilla 2.16
Priority: -- → P3
Keywords: patch, review
->New (old by now?) product Bugzilla
Assignee: tara → justdave
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
*** Bug 102689 has been marked as a duplicate of this bug. ***
Attached file index.cgi - Uses a template (obsolete) (deleted) —
Attached file The aforementioned template (obsolete) (deleted) —
Summary: configurable index page [patch] → configurable index page (using Template Toolkit)
Attachment #34031 - Attachment is obsolete: true
Attachment #34032 - Attachment is obsolete: true
-> me
Assignee: justdave → jake
Status: NEW → ASSIGNED
Making this bug block the templatization tracker bug.
Blocks: bz-template
Jake, this looks pretty good. I have some questions: # Colon-separated list of directories containing templates. INCLUDE_PATH => "template/default" , Is this correct? IOW, if the user wants to override this, does he have to hack the file or it automatic? (Sorry for being clueless) $vars->{'username'} = $::COOKIE{'Bugzilla_login'} || ''; $vars->{'subst'} = { 'userid' => $::COOKIE{'Bugzilla_login'} }; Why do you check for an invalid cookie first, and on the second line, ignore the test? Any specific reason? Why not $vars->{'subst'} = { 'userid' => $vars->{'username'} }; perhaps? <SCRIPT LANGUAGE="JavaScript"> This should be <SCRIPT TYPE="application/x-javascript"> var rv = window.confirm ("This page is enhanced for use with Netscape 6. " + "Would you like to upgrade now?"); ROTFL Why not mention Mozilla? Looks good to me, if these are problems and are fixed r=kiko
(spam, sorry, stupid: vote for bug 34488 to solve this once and for all)
>Is this correct? IOW, if the user wants to override this, does he have to hack >the file or it automatic? (Sorry for being clueless) Well, I copied that from attachment.cgi :) >Why do you check for an invalid cookie first, and on the second line, ignore the >test? Any specific reason? Why not Not sure what I was thinking... >This should be <SCRIPT TYPE="application/x-javascript"> ... >ROTFL Why not mention Mozilla? I took that bit from Netscape's web site w/only minor modifications. I assume the first is for HTML 4 compliance? As for mentioning Mozilla, I guess I have no opionion either way... I've heard it said that Mozilla is really for the devolopers and end users are better off w/a distribution (such as Netscape 6).
Attached file index.cgi - v2 (obsolete) (deleted) —
Attached file index.atml - v2 (obsolete) (deleted) —
Attachment #52172 - Attachment is obsolete: true
Attachment #52172 - Attachment is patch: false
Attachment #52173 - Attachment is obsolete: true
Attachment #52173 - Attachment is patch: false
Comment on attachment 53096 [details] index.cgi - v2 > INCLUDE_PATH => "template/default" , This should be: INCLUDE_PATH => "template/custom:template/default" , The way it is in attachment.cgi is a mistake.
Attachment #53096 - Flags: review-
Personally I think this is annoying, because I find it very useful to have a static page available. This helps me see whether problems during setup are webserver permissions problems or execute permissions problems or SQL server problems or what. Do we ship other static HTML files I could use for this instead? Gerv
Gerv, there are numerous static files... the entire docs/ sub-tree for example (there are also many in the "bugzilla root" directory.
Attached file index.cgi - v3 (obsolete) (deleted) —
Attachment #53096 - Attachment is obsolete: true
*** Bug 15967 has been marked as a duplicate of this bug. ***
Comment on attachment 53395 [details] index.cgi - v3 ># Suppress silly "used only once" warnings >sub index_cgi_silliness { > my $zz; > $zz = %::COOKIE; >} This is not necessary - there are several bugs, with patches attached, that show how to use "use vars" instead. And some comments on the template: > Forget the currently stored login Can't we just say "Logout" like everyone else? > ...explaining all about bugzilla. _B_ugzilla :-) You main page doesn't look quite like the default one; e.g. the black border around the bug image, which makes it look a lot neater. And some of the words are missing from the top. Why is that? Gerv
I tried to change several of those to use "use vars" in the past, and was never able to get "use vars" to work with global variables.
IIRC, I also tried to "use vars" and it balked. > Can't we just say "Logout" like everyone else? Um, sure :) > the black border around the bug image, which makes it look a lot neater Well, ok... > And some of the words are missing from the top. Why is that? The "This is Bugzilla" words we duplicated less than half a screen down (in the page footer) when using defaults, so I didn't put them at the top of the main page.
Attached file index.atml - v3 (obsolete) (deleted) —
Attachment #53099 - Attachment is obsolete: true
Comment on attachment 53395 [details] index.cgi - v3 r=kiko Thanks Jake.
Attachment #53395 - Flags: review+
Comment on attachment 53632 [details] index.atml - v3 r=kiko Let's get this in and fix anything that falls in later :)
Attachment #53632 - Flags: review+
Comment on attachment 53395 [details] index.cgi - v3 r=gerv. Gerv
Attachment #53395 - Flags: review+
Comment on attachment 53632 [details] index.atml - v3 r=gerv. Gerv
Attachment #53632 - Flags: review+
Gerv/Chistrian, in order some of the things in this patch refer to QuickSearch patch or the sidebar.cgi from bug 37339... can I get review on that, too?
Blocks: 15967
Comment on attachment 54155 [details] [diff] [review] Optionally create an index.html to redirect to index.cgi I like what's in this patch so far, but I want more! :) People who are updating via cvs may suddenly be surprised when their existing index.html file disappears when they cvs update after we cvs remove it... I believe cvs will move their existing file to ".#index.html.(version)" where (version) is the RCS revision number for the last version they had from cvs before they updated. The checksetup.pl script should, *before* checking localconfig vars, look for the presense of a file by such a name, and indicate that their old index.html file has been moved there if it finds one. Also indicate that they can remove that file to get rid of the warning every time they run checksetup.pl.
Attachment #54155 - Flags: review-
ok.... ignore everything I said above... that's now how it works. I did some testing... on installation 1 I added a file called "testfile" which simply contained a single line of text. > cvs add textfile > cvs commit Checking in testfile; /cvsroot/syndiboard2/testfile,v <-- testfile initial revision: 1.1 done on installation 2, I did a cvs update, then removed the file: > cvs up cvs update: Updating . U testfile > rm testfile > cvs remove testfile cvs remove: scheduling `testfile' for removal cvs remove: use 'cvs commit' to remove this file permanently > cvs commit Removing testfile; /cvsroot/syndiboard2/testfile,v <-- testfile new revision: delete; previous revision: 1.1 done OK, back to installation 1... I modified testfile so it had different text in it than was there when I originally committed it... then did a cvs update: > cvs up cvs server: Updating . RCS file: /cvsroot/syndiboard2/Attic/testfile,v retrieving revision 1.1 retrieving revision 1.2 Merging differences between 1.1 and 1.2 into testfile testfile already contains the differences between 1.1 and 1.2 > ls -l testfile -rw------- 1 dave unknown 30 Oct 25 23:11 testfile it's still there.... > cvs diff -u testfile cvs server: testfile is a new entry, no comparison available > cvs up cvs server: Updating . M testfile
I did an expiriment very similar to what you did as far as having two installations... This is what happened when README.txt was locally modified: [intranet@webserv3 spit]$ cvs -q update -dP cvs server: conflict: README.txt is modified but no longer in the repository C README.txt [intranet@webserv3 spit]$ ll total 12 drwxrwxrwx 2 intranet nobody 4096 Oct 26 08:33 CVS -rw-r--r-- 1 intranet nobody 1021 Oct 26 08:37 README.txt drwxrwxrwx 4 intranet nobody 4096 Oct 26 08:37 Vb6 [intranet@webserv3 spit]$ cd CVS [intranet@webserv3 CVS]$ cat Entries D/Vb6//// /README.txt/1.11/Fri Oct 26 12:33:28 2001// [intranet@webserv3 CVS]$ And when it wasn't: [intranet@webserv3 spit]$ cvs -q update -dP cvs server: warning: README.txt is not (any longer) pertinent [intranet@webserv3 spit]$ ll total 8 drwxrwxrwx 2 intranet nobody 4096 Oct 26 08:38 CVS drwxrwxrwx 4 intranet nobody 4096 Oct 26 08:38 Vb6 [intranet@webserv3 spit]$ cd CVS [intranet@webserv3 CVS]$ cat Entries D/Vb6//// [intranet@webserv3 CVS]$
Attachment 55231 [details] [diff] on bug 37339 contains my attempt to detect the situation of index.html not being removed because it's modified and warn the user. What it does is look for "index.cgi" in the .html file and if it doesn't find it, warns the user (we dont' want to warn them if they chose to have checksetup.pl create the index.html file to redirect them to index.cgi :)
The CGI and page looks OK on a quick scan, but the CGI should run in taint mode.
Depends on: 37339
> Attachment 55231 [details] [diff] on bug 37339 contains my attempt to detect the situation of > index.html not being removed because it's modified and warn the user. What it > does is look for "index.cgi" in the .html file and if it doesn't find it, warns > the user (we dont' want to warn them if they chose to have checksetup.pl create > the index.html file to redirect them to index.cgi :) If CVS/Entries exists, can't you just grep for ^index.html in that? It leaves some text in there so that you get teh conflict message.
>If CVS/Entries exists, can't you just grep for ^index.html in that? It leaves >some text in there so that you get teh conflict message. I thought about doing that, but then I relized that if the did: cvs update ./checksetup.pl <some work to put index.html changes into template/custom/index.atml> rm index.html vi localconfig <change $index_html to 1> ./checksetup.pl Then index.html will still exist in CVS/Entries even though they've now properly set it up to use the template (but because of their webserver configuration they needed the index.html redirect page). Of course, I suppose in this situation cvs would still complain about the localally modified index.html. The other situation is kinda the opposit, where checking CVS/Entires wouldn't catch the fact that an error should be thrown. If the user grabs the 2.16 (I'm assuming) tarball, untars it and copies index.html from their 2.14 (or older) install, then index.html will no longer exist in CVS/Entires, but they still wouldn't be using index.cgi.
Hmm. We could just hack .htaccess to prefer index.cgi, but that won't get all cases.
Attachment #53395 - Attachment is obsolete: true
Attachment #53632 - Attachment is obsolete: true
Attachment #54155 - Attachment is obsolete: true
Ahhh, bannerHTML and footerHTML automatically... :-) I did something like this with my install. I think I took index.cgi from a patch of a bug long ago--Dave Lawarance did it IIRC (I could submit it if anyone cared, but it looks like y'all are pretty far along on this already). Anyway, I might add that I think it's a good idea not to totally kill index.html, that might make life a little harder on some people unneccessarily. (Yes I know I could avoid the work by just adding another file name to the default files list [DirectoryIndex for Apache], but why should I?) I avoided the problem by changing index.html to: <html> <meta http-equiv="refresh" content="0; URL=index.cgi"> </html> Because of that, no bookmarks by anyone were affected, no Apache config files had to be changed, and no animals were sacrified. ;-)
Kevin, the idea is that checksetup.pl will optionally create an index.html that will redirect to index.cgi. The latest patches for this are on bug 37339 (because of the interdependencies... which is the while reason I started work on this bug :)
Priority: P3 → P1
blocks a blocker, so it's a blocker.
Severity: enhancement → blocker
Bug 37339 had been checked in which included the fix for this bug in the patch.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: