parameterize body tag attribute values

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
16 years ago
6 years ago

People

(Reporter: myk, Assigned: myk)

Tracking

2.15
Bugzilla 2.16

Details

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
The check-in for bug 147272 added the following code to the body tag in
header.html.tmpl:

[% "bgcolor=\"#FFFFFF\"" UNLESS body_attributes.search('(?:^|\s)bgcolor=') %]

A better approach would be to make bgcolor a template variable.  Then the code
could look like this:

[% DEFAULT bgcolor = "#FFFFFF" %]
<body bgcolor="[% bgcolor %]" ...>

The same should be done for the onload attribute, which is currently the only
attribute that ever gets passed into header.html.tmpl in the "body_attributes"
variable.
(Assignee)

Comment 1

16 years ago
Created attachment 85630 [details] [diff] [review]
patch v1: fixes the problem

Comment 2

16 years ago
You seem to propose removing body_attributes entirely. Is this wise? What if
there are needs other than bgcolor and onload? Is there a reason to remove
body_attributes? Shouldn't bgcolor and onload be mentioned in the interface
comments? 

And even more importantly, do we want this for 2.16 to stabilize the interface
before we release?
(Assignee)

Comment 3

16 years ago
Yes, removing body_attributes is wise.  If template developers need to add more
attributes to the body tag they can and should use additional parameters to do
it.  Yes, the parameters should be mentioned in the interface comments.  Yes, we
want this for 2.16.

Comment 4

16 years ago
Well, marking 2.16 then.

About body_attributes: I didn't quite get your point. While template authors
certainly _can_ create additional parameters, why is it good for them to do so?
I mean, if I want to customize my site to have a "ontrackballrollstoofast" event
supported by future browsers, I have to whack both the header and my custom
template. It's not a big job, but still: if header.html.tmpl does nothing (apart
from printing, of course) with the parameters anyway, do they all have to be
named specifically by the attribute?

Not an objection, just a request for more explanation (a usable reference to
past discussion will do, of course). :-)
Target Milestone: --- → Bugzilla 2.16
Keywords: patch, review
Comment on attachment 85630 [details] [diff] [review]
patch v1: fixes the problem

r= justdave
Attachment #85630 - Flags: review+
Comment on attachment 85630 [details] [diff] [review]
patch v1: fixes the problem

r=bbaetz

If we find that we want to add more stuff, for some reason, we can have an
extra_attribtues param later.

Please fix up the INTERFACE comments before checkin.
Attachment #85630 - Flags: review+
(Assignee)

Comment 7

16 years ago
Jouni- If one person wants an additional attribute, most likely other people
want it too, and it's a much cleaner and more transparent interface to make it a
parameter.  If, however, we find that lots of custom template authors have need
for funky attributes that no one else will want, like your
"ontrackballrollstoofast" example, then we can do what bbaetz said and add an
"extra_attributes" parameter or, even better, make body attributes into a hash
to which any custom template can add its own custom values.
(Assignee)

Updated

16 years ago
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

16 years ago
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.9; previous revision: 1.8
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.7; previous revision: 1.6
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.9; previous revision: 1.8
done

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.2.4; previous revision: 1.5.2.3
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.2.2; previous revision: 1.5.2.1
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.2.1; previous revision: 1.8
done

QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.