140993 - Pass javascript correctly in PutHeader()

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[mental]

patch incoming...

Comment 1

[mental]

Attachment: simple fix

Comment 2

[mental]

Attachment: better fix

Comment 3

[Gervase Markham [:gerv]]

Are you sure that users of this parameter do not provide their own script tags?

If you are sure about that, how come everything's working at the moment?

Gerv

Comment 4

[mental]

Everything is working because nobody is using it!  Passing in the script tags
doesn't make a lot of sense, as they should always be the same for a Javascript
script.

Comment 5

[Gervase Markham [:gerv]]

I agree that your way is better; I'm just not convinced that no-one is using it. 

<checks>

In fact, someone _is_ using it - but they are using it in the wrong way, and if
you fix this, it'll break. See CGI.pl. You'll need to work out what's going on
there and fix it up too.

Gerv

Comment 6

[Gervase Markham [:gerv]]

Targetting provisionally at 2.16, if Mental can come up with a patch soon :-)

Gerv

Comment 7

[Gervase Markham [:gerv]]

Attachment: Patch v.1

This patch does the following:

- fixes this bug
- deals with some fallout whereby the old jscript parameter was being abused,
and provides a new parameter ("header_html") to pass arbitrary stuff into the
header
- renames the "jscript" parameter to "javascript" to avoid confusion
- renames "extra" to "body_attributes" for the same reason

- adds "migration" comments for admins migrating from the old Param-based HTML
- removes a couple of unneeded parameters from the PutHeader function.
- removes redundant defaults at the top of the header template
- copies the "bannerhtml" Param into the template, where it belongs. This is a
straight copy.

This makes the header template, IMO, shippable.

Gerv

Comment 8

[mental]

Gerv, this is more ambitious than I really needed.  Perhaps that is why I didn't
understand your comments above.  I can't see where you are passing header_html
or javascript in your patch, though.  Giving this to you...

Comment 9

[Gervase Markham [:gerv]]

It is a little more ambitious, but nothing complicated is going on in the patch,
and it cleans up some stuff that really need cleaning.

Gerv

Comment 10

[Bradley Baetz (:bbaetz)]

Comment on attachment 81836 [details] [diff] [review]
Patch v.1

>Index: template/en/default/global/header.html.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v
>retrieving revision 1.3
>diff -u -r1.3 header.html.tmpl
>--- template/en/default/global/header.html.tmpl	19 Apr 2002 22:38:01 -0000	1.3
>+++ template/en/default/global/header.html.tmpl	1 May 2002 06:56:22 -0000

> 
> [% DEFAULT
>-  title = ""
>   h1 = title
>-  h2 = ""
>-  extra = ""
>-  jscript = ""
>-  style = ""
>-  message = ""
> %]

> [% IF h1 || h2 %]
>     <table border="0" cellspacing="0">

The stuff below this could be split up - some of the tags are only needed for
one of h1 or h2, not both.
That doesn't have to be done now, though.

>Index: template/en/default/search/search.html.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search.html.tmpl,v
>retrieving revision 1.6
>diff -u -r1.6 search.html.tmpl
>--- template/en/default/search/search.html.tmpl	26 Apr 2002 22:16:34 -0000	1.6
>+++ template/en/default/search/search.html.tmpl	1 May 2002 06:56:24 -0000
>@@ -21,7 +21,7 @@
> 
> [% PROCESS global/header.html.tmpl 
>   title = "Search for bugs"
>-  extra = " onLoad=\"selectProduct(document.forms['queryform']);\""
>+  body_attributes = " onLoad=\"selectProduct(document.forms['queryform']);\""
> %]

There is an extra space here.

r=bbaetz.

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 81836 [details] [diff] [review]
Patch v.1

>Index: template/en/default/global/header.html.tmpl
>===================================================================
<...>
>+    <center>
>+      <font size="-1">Bugzilla version [% Param("version") %]</font>
>+    </center>

Please use <small> instead of <font size="-1">

>Index: template/en/default/search/search.html.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search.html.tmpl,v
<...>
>-  extra = " onLoad=\"selectProduct(document.forms['queryform']);\""
>+  body_attributes = " onLoad=\"selectProduct(document.forms['queryform']);\""

And though the code is pre-existing and not introduced in this patch, please
use "onload" and not "onLoad".

r=caillon

Comment 12

[Gervase Markham [:gerv]]

Fixed :-)

Checking in template/en/default/global/header.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v
 <--  header.html.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--
 edit.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.151; previous revision: 1.150
done
Checking in template/en/default/search/search.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search.html.tmpl,v
 <--  search.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/attachment/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/create.html.tmpl,v
 <--  create.html.tmpl
new revision: 1.5; previous revision: 1.4
done

Gerv