Closed Bug 148179 Opened 22 years ago Closed 22 years ago

parameterize body tag attribute values

Categories

(Bugzilla :: Bugzilla-General, defect)

Product:
Bugzilla
Component:
Bugzilla-General
Version:
2.15
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 2.16

People

(Reporter: myk, Assigned: myk)

Details

Attachments

(1 file)

patch v1: fixes the problem
2.56 KB, patch
justdave
: review+
bbaetz
: review+
Details | Diff | Splinter Review 
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.
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?
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.
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+
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.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → 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.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.

Attachment

General

Creator:
Created:
Updated:
Size: