148679 - Permit multiple stylesheets in header template

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[Joel Peshkin]

Bugzilla 2.16rc1 templates assume that there will be a maximum of one style_url
for a page.  Change this to a list of style_urls.  (One cool thing, a string
seems to behave as a list of one item, so it is backward-compatible with any
templates that don;t know this can be a list)

Comment 1

[Joel Peshkin]

Attachment: Change to header.html.tmpl to permit multiple stylesheets

This patch permits the calling template to pass a list of stylesheets instead
of a single stylesheet to header.html.tmpl.   All other things being equal, the
LAST on the list has precedence.

Comment 2

[Bradley Baetz (:bbaetz)]

Comment on attachment 85986 [details] [diff] [review]
Change to header.html.tmpl to permit multiple stylesheets

Is the fact that a scalar is a one-element list something we can count on, or
is it just an artifact of how stuff is stored internally by TT?

Comment 3

[Joel Peshkin]

You're probably right. I reread all the template documentation and the ability
of FOREACH to iterate once if it has a string instead of a list doesn't seem to
be guaranteed anywhere.   As far as I can tell, the only place that would have
to be updated to have an extra set of square brackets is list/list.html.tmpl 

It probably pays to make the change there as well.  

diff -u -r1.4 list.html.tmpl
--- list.html.tmpl      21 May 2002 17:53:40 -0000      1.4
+++ list.html.tmpl      3 Jun 2002 03:02:52 -0000
@@ -24,7 +24,7 @@
 [%############################################################################%
]

 [% DEFAULT title = "Bug List" %]
-[% style_url = "css/buglist.css" %]
+[% style_url = [ "css/buglist.css" ] %]
 [% qorder = order FILTER url_quote IF order %]


                                                    

Comment 4

[Jouni Heikniemi]

Comment on attachment 85986 [details] [diff] [review]
Change to header.html.tmpl to permit multiple stylesheets

>Index: template/en/default/global/header.html.tmpl
>===================================================================
>+  # style_url: list. list of CSS styles.

This parameter should be "style_urls" if we're going 
to change list.html.tmpl anyway; otherwise the added [ ] around
a single url won't make much sense to a casual reader.

As for that comment quoted above: how about "List of URLs to CSS 
stylesheets." The current wording can be understood as a list of
actual CSS rules, especially when separated from the context.

>+      [% FOREACH this_style_url = style_url %]

... and then you could write this as FOREACH style_url = style_urls, 
which seems to be some sort of a convention here these days.

Comment 5

[Joel Peshkin]

I certainly could make this style_urls and make the change in list.html as well,
but that would break compatibility for anyone who had used stylesheets in
customizations.  Before I change style_url to style_urls in the interface, does
anyone have a strong opinion?

Comment 6

[Bradley Baetz (:bbaetz)]

If its a list, it should be plural. I'm curious why you'd need this, though.
Where would this be used?

Comment 7

[Joel Peshkin]

The origin of this is my attempts to color-code the comments.  When generating
the comments in the edit template, I wanted to code both the font and the
background of that section.  When using the same routines to generate the
comments formatted for printing, I want to code the font differrently and not
apply that the the background color.

Ordinarily, I would have just used 2 differrent stylesheets.  However, that
would mean that the first time I decide to use a stylesheet, I am precluding
anyone else from ever using a differrent one.   Gerv and I exchanged some email
and agreed that I should create a, hopefully very localized, patch.

A good case in point for this is someone who wants to enlarge all the fonts and
puts 
P,TR,BODY { font-size : 150% }   
in a stylesheet.  If differrent Bugzilla components use differrent stylesheets,
then this would have to be inserted into evry stylesheet.  By permitting
multiple stylesheets, however, there can be a local stylesheet listed in
addition to the stylesheet designed by the template authout.

Comment 8

[Gervase Markham [:gerv]]

> I certainly could make this style_urls and make the change in list.html as well,
> but that would break compatibility for anyone who had used stylesheets in
> customizations.

This is why we are doing it now, rather than later :-) Template interfaces are
still fluid, as we haven't released.

Gerv

Comment 9

[Joel Peshkin]

Attachment: Multiple stylesheet patch using "style_urls"

OK... style_urls it is

Comment 10

[Gervase Markham [:gerv]]

Comment on attachment 86215 [details] [diff] [review]
Multiple stylesheet patch using "style_urls"

>+  # style_urls: list. list of CSS styles.

"list of URLs to CSS style sheets."

Other than that, r=gerv.

Gerv

Comment 11

[Jouni Heikniemi]

Comment on attachment 86215 [details] [diff] [review]
Multiple stylesheet patch using "style_urls"

Umm yeah, I agree with Gerv. Make a new patch with the mentioned comment fix
and you have r=jouni on it.

Comment 12

[Joel Peshkin]

Attachment: Revision to 86215 with comment change from review

Comment 13

[Jouni Heikniemi]

Comment on attachment 86220 [details] [diff] [review]
Revision to 86215 with comment change from review

>+  # style_urls: list. List URLs to CSS style sheets.

Nit: Make that 'List of URLs to CSS style sheets.'
		     --
:-)

Comment 14

[Joel Peshkin]

Attachment: Patch with clearer comment - ficntionally identical to 86220

Nit covered :-)

Comment 15

[Jouni Heikniemi]

Comment on attachment 86221 [details] [diff] [review]
Patch with clearer comment - ficntionally identical to 86220

Ok then :-)

r=jouni, and there was a r=gerv two attachments up, so I'll mark that as well.
Ready for checkin.

Comment 16

[Gervase Markham [:gerv]]

Is this meant for the 2.16 branch?

Gerv

Comment 17

[Jouni Heikniemi]

Gerv: Per bug 69654 comment 27 and comment 8 on this bug, I'd say that this
should be in 2.16. Reconsider yourself, though; both the mentioned sources are
written by you :-) I'd do it now because this involves an interface change.

Marking 2.16, bump if you change your mind.

Comment 18

[Dave Miller [:justdave]]

As long as the patch is available and works there, let's go ahead and put this
in 2.16.  It won't hurt anything and it's already here.

Comment 19

[Dave Miller [:justdave]]

checking in on behalf of Joel since he doesn't have checkin privs to my knowledge...

HEAD:
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.10; previous revision: 1.9
done
Checking in template/en/default/list/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v 
<--  list.html.tmpl
new revision: 1.5; previous revision: 1.4
done

BUGZILLA-2_16-BRANCH:
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.5; previous revision: 1.5.2.4
done
Checking in template/en/default/list/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v 
<--  list.html.tmpl
new revision: 1.3.2.2; previous revision: 1.3.2.1
done