140103 - Templatisation odds and ends

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Gervase Markham [:gerv]]

I just had a change to grep the sources for places where templatisation is still
needed for user-facing stuff. We can't really avoid doing it; not having it
causes bugs like bug 137121, and it causes trouble for our localisers.

The good news is that it's not all that complicated - just a little bit
long-winded. It's mostly in CGI.pl. I'll split the patch into several pieces to
make it easier to review.

Gerv

Comment 1

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Templatise some of the last few things in CGI.pl. There's still some more to
do, but I don't want this patch to get too big.

Gerv

Comment 2

[Gervase Markham [:gerv]]

Another 2.16 bug. Well, at least I provide patches when I file them :-)

Gerv

Comment 3

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Missed a file by forgetting to save it before generating the diff.

Gerv

Comment 4

[Bradley Baetz (:bbaetz)]

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

This looks fine, except...

> sub PutHeader {

>-
>-    if (Param("shutdownhtml")) {
>-        # If we are dealing with the params page, we want
>-        # to ignore shutdownhtml
>-        if ($0 !~ m:[\\/](do)?editparams.cgi$:) {
>-            print "<p>\n";
>-            print Param("shutdownhtml");
>-            exit;
>-        }
>-    }
>+    ($vars->{'title'}, $vars->{'h1'}, $vars->{'h2'}, 
>+     $vars->{'extra'}, $vars->{'jscript'}) = (@_);
>+     
>+    $::template->process("global/header.html.tmpl", $::vars)
>+      || ThrowTemplateError($::template->error());
> }

where does your code special case the params page?

Comment 5

[Gervase Markham [:gerv]]

bbaetz: the shutdownhtml stuff is now done at the top of CGI.pl. According to
lxr, Myk made this change on 3rd Feb this year. So, it doesn't need to be in
ShowHeader() any more.

Gerv

Comment 6

[Bradley Baetz (:bbaetz)]

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

ok. r=bbaetz, then. Looks fine, and a random testing sample didn't have any
problems.

Comment 7

[Dave Miller [:justdave]]

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

why are we removing SyncAnyPendingShadowChanges() from PutFooter?  Won't that
break installs using shadowdb?

Comment 8

[Dave Miller [:justdave]]

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

ok, you added it to the template (found the other bug on it).  I don't think
that's right (Syncing the shadow database is a code thing and not a user
interface thing, and we shouldn't be depending on a template to call it).  But
that's another issue, and this patch is good for what it does.

r= justdave

Comment 9

[Gervase Markham [:gerv]]

Fixed.

Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.149; previous revision: 1.148
done
Checking in template/en/default/bug/process/results.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/process/results.html.tmpl,v
 <--  results.html.tmpl
new revision: 1.2; previous revision: 1.1
done

Gerv