From 10f4954b4530a67d88c66ba8e397ee81487f4752 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Tue, 20 Jul 2010 20:34:41 -0700 Subject: Bug 428313: Properly expire the browser's CSS and JS cache when there are new versions of those files. This also eliminates single-file skins and should also allow Extensions to have skins. r=glob, a=mkanat --- .htaccess | 11 ++ Bugzilla/Install/Filesystem.pm | 59 +++++--- Bugzilla/Template.pm | 120 +++++++++++++++++ docs/en/xml/installation.xml | 2 +- mod_perl.pl | 2 +- skins/README | 20 +++ template/en/default/global/header.html.tmpl | 201 ++++++++++------------------ template/en/default/setup/strings.txt.pl | 2 + 8 files changed, 267 insertions(+), 150 deletions(-) create mode 100644 skins/README diff --git a/.htaccess b/.htaccess index 72a96e064..d4436e563 100644 --- a/.htaccess +++ b/.htaccess @@ -2,3 +2,14 @@ deny from all + + ExpiresActive On + # According to RFC 2616, "1 year in the future" means "never expire". + # We change the name of the file's URL whenever its modification date + # changes, so browsers can cache any individual JS or CSS URL forever. + # However, since all JS and CSS URLs involve a ? in them (for the changing + # name) we have to explicitly set an Expires header or browsers won't + # *ever* cache them. + ExpiresDefault "now plus 1 years" + Header append Cache-Control "public" + diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm index db55576a4..20bd021ef 100644 --- a/Bugzilla/Install/Filesystem.pm +++ b/Bugzilla/Install/Filesystem.pm @@ -284,20 +284,6 @@ sub FILESYSTEM { contents => '' }, ); - # Each standard stylesheet has an associated custom stylesheet that - # we create. Also, we create placeholders for standard stylesheets - # for contrib skins which don't provide them themselves. - foreach my $skin_dir ("$skinsdir/custom", <$skinsdir/contrib/*>) { - next if basename($skin_dir) =~ /^cvs$/i; - foreach my $base_css (<$skinsdir/standard/*.css>) { - _add_custom_css($skin_dir, basename($base_css), \%create_files); - } - foreach my $dir_css (<$skinsdir/standard/*/*.css>) { - $dir_css =~ s{.+?([^/]+/[^/]+)$}{$1}; - _add_custom_css($skin_dir, $dir_css, \%create_files); - } - } - # Because checksetup controls the creation of index.html separately # from all other files, it gets its very own hash. my %index_html = ( @@ -455,20 +441,53 @@ EOT print "Removing duplicates directory...\n"; rmtree("$datadir/duplicates"); } + + _remove_empty_css_files(); + _convert_single_file_skins(); } -# A simple helper for creating "empty" CSS files. -sub _add_custom_css { - my ($skin_dir, $path, $create_files) = @_; - $create_files->{"$skin_dir/$path"} = { perms => WS_SERVE, contents => <{'skinsdir'}; + foreach my $css_file (glob("$skinsdir/custom/*.css"), + glob("$skinsdir/contrib/*/*.css")) + { + _remove_empty_css($css_file); + } +} + +# A simple helper for the update code that removes "empty" CSS files. +sub _remove_empty_css { + my ($file) = @_; + my $basename = basename($file); + my $empty_contents = <; } + if ($file_contents eq $empty_contents) { + print install_string('file_remove', { name => $file }), "\n"; + unlink $file or warn "$file: $!"; + } }; } +# We used to allow a single css file in the skins/contrib/ directory +# to be a whole skin. +sub _convert_single_file_skins { + my $skinsdir = bz_locations()->{'skinsdir'}; + foreach my $skin_file (glob "$skinsdir/contrib/*.css") { + my $dir_name = $skin_file; + $dir_name =~ s/\.css$//; + mkdir $dir_name or warn "$dir_name: $!"; + _rename_file($skin_file, "$dir_name/global.css"); + } +} + sub create_htaccess { _create_files(%{FILESYSTEM()->{htaccess}}); @@ -492,7 +511,7 @@ sub create_htaccess { sub _rename_file { my ($from, $to) = @_; - print "Renaming $from to $to...\n"; + print install_string('file_rename', { from => $from, to => $to }), "\n"; if (-e $to) { warn "$to already exists, not moving\n"; } diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index 2c2d402a3..ffd702e62 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -358,6 +358,121 @@ sub get_bug_link { return qq{$pre$link_text$post}; } +##################### +# Header Generation # +##################### + +# Returns the last modification time of a file, as an integer number of +# seconds since the epoch. +sub _mtime { return (stat($_[0]))[9] } + +sub mtime_filter { + my ($file_url) = @_; + my $cgi_path = bz_locations()->{'cgi_path'}; + my $file_path = "$cgi_path/$file_url"; + return "$file_url?" . _mtime($file_path); +} + +# Set up the skin CSS cascade: +# +# 1. YUI CSS +# 2. Standard Bugzilla stylesheet set (persistent) +# 3. Standard Bugzilla stylesheet set (selectable) +# 4. All third-party "skin" stylesheet sets (selectable) +# 5. Page-specific styles +# 6. Custom Bugzilla stylesheet set (persistent) +# +# "Selectable" skin file sets may be either preferred or alternate. +# Exactly one is preferred, determined by the "skin" user preference. +sub css_files { + my ($style_urls, $yui, $yui_css) = @_; + + # global.css goes on every page, and so does IE-fixes.css. + my @requested_css = ('skins/standard/global.css', @$style_urls, + 'skins/standard/IE-fixes.css'); + + my @yui_required_css; + foreach my $yui_name (@$yui) { + next if !$yui_css->{$yui_name}; + push(@yui_required_css, "js/yui/assets/skins/sam/$yui_name.css"); + } + unshift(@requested_css, @yui_required_css); + + my @css_sets = map { _css_link_set($_) } @requested_css; + + my %by_type = (standard => [], alternate => {}, skin => [], custom => []); + foreach my $set (@css_sets) { + foreach my $key (keys %$set) { + if ($key eq 'alternate') { + foreach my $alternate_skin (keys %{ $set->{alternate} }) { + my $files = $by_type{alternate}->{$alternate_skin} ||= []; + push(@$files, $set->{alternate}->{$alternate_skin}); + } + } + else { + push(@{ $by_type{$key} }, $set->{$key}); + } + } + } + + return \%by_type; +} + +sub _css_link_set { + my ($file_name) = @_; + + my $standard_mtime = _mtime($file_name); + my %set = (standard => $file_name . "?$standard_mtime"); + + # We use (^|/) to allow Extensions to use the skins system if they + # want. + if ($file_name !~ m{(^|/)skins/standard/}) { + return \%set; + } + + my $user = Bugzilla->user; + my $cgi_path = bz_locations()->{'cgi_path'}; + my $all_skins = $user->settings->{'skin'}->legal_values; + my %skin_urls; + foreach my $option (@$all_skins) { + next if $option eq 'standard'; + my $skin_file_name = $file_name; + $skin_file_name =~ s{(^|/)skins/standard/}{skins/contrib/$option/}; + if (my $mtime = _mtime("$cgi_path/$skin_file_name")) { + $skin_urls{$option} = $skin_file_name . "?$mtime"; + } + } + $set{alternate} = \%skin_urls; + + my $skin = $user->settings->{'skin'}->{'value'}; + if ($skin ne 'standard' and defined $set{alternate}->{$skin}) { + $set{skin} = delete $set{alternate}->{$skin}; + } + + my $custom_file_name = $file_name; + $custom_file_name =~ s{(^|/)skins/standard/}{skins/custom/}; + if (my $custom_mtime = _mtime("$cgi_path/$custom_file_name")) { + $set{custom} = $custom_file_name . "?$custom_mtime"; + } + + return \%set; +} + +# YUI dependency resolution +sub yui_resolve_deps { + my ($yui, $yui_deps) = @_; + + my @yui_resolved; + foreach my $yui_name (@$yui) { + my $deps = $yui_deps->{$yui_name} || []; + foreach my $dep (reverse @$deps) { + push(@yui_resolved, $dep) if !grep { $_ eq $dep } @yui_resolved; + } + push(@yui_resolved, $yui_name) if !grep { $_ eq $yui_name } @yui_resolved; + } + return \@yui_resolved; +} + ############################################################################### # Templatization Code @@ -647,6 +762,8 @@ sub create { html_light => \&Bugzilla::Util::html_light_quote, email => \&Bugzilla::Util::email_filter, + + mtime_url => \&mtime_filter, # iCalendar contentline filter ics => [ sub { @@ -769,6 +886,9 @@ sub create { Bugzilla->fields({ by_name => 1 }); return $cache->{template_bug_fields}; }, + + 'css_files' => \&css_files, + yui_resolve_deps => \&yui_resolve_deps, # Whether or not keywords are enabled, in this Bugzilla. 'use_keywords' => sub { return Bugzilla::Keyword->any_exist; }, diff --git a/docs/en/xml/installation.xml b/docs/en/xml/installation.xml index 2b8e1b878..b498acb81 100644 --- a/docs/en/xml/installation.xml +++ b/docs/en/xml/installation.xml @@ -1044,7 +1044,7 @@ max_allowed_packet=4M AddHandler cgi-script .cgi Options +Indexes +ExecCGI DirectoryIndex index.cgi - AllowOverride Limit + AllowOverride Limit FileInfo Indexes </Directory> diff --git a/mod_perl.pl b/mod_perl.pl index a21d5d725..32fe82ccf 100644 --- a/mod_perl.pl +++ b/mod_perl.pl @@ -74,7 +74,7 @@ PerlChildInitHandler "sub { srand(); }" $sizelimit PerlOptions +ParseHeaders Options +ExecCGI - AllowOverride Limit + AllowOverride Limit FileInfo Indexes DirectoryIndex index.cgi index.html EOT diff --git a/skins/README b/skins/README new file mode 100644 index 000000000..111c00f03 --- /dev/null +++ b/skins/README @@ -0,0 +1,20 @@ +There are three directories here, standard/, custom/, and contrib/. + +standard/ holds the standard stylesheets. These are used no matter +what skin the user selects. If the user selects the "Classic" skin, +then *only* the standard/ stylesheets are used. + +contrib/ holds "skins" that the user can select in their preferences. +skins are in directories, and they contain files with the same names +as the files in skins/standard/. Simply putting a new directory +into the contrib/ directory adds a new skin as an option in users' +preferences. + +custom/ allows you to locally override the standard/ and contrib/ CSS. +If you put files into the custom/ directory with the same names as the CSS +files in skins/standard/, you can override the standard/ and contrib/ +CSS. For example, if you want to override some CSS in +skins/standard/global.css, then you should create a file called "global.css" +in custom/ and put some CSS in it. The CSS you put into files in custom/ will +be used *in addition* to the CSS in skins/standard/ or the CSS in +skins/contrib/. It will apply to every skin. diff --git a/template/en/default/global/header.html.tmpl b/template/en/default/global/header.html.tmpl index 721afd7af..549fd538f 100644 --- a/template/en/default/global/header.html.tmpl +++ b/template/en/default/global/header.html.tmpl @@ -64,6 +64,14 @@ datatable => ['json', 'connection', 'datasource', 'element'], } %] +[%# These are JS URLs that are *always* on the page and come before + # every other JS URL. + #%] +[% SET starting_js_urls = [ + "js/yui/yahoo-dom-event/yahoo-dom-event.js", + "js/yui/cookie/cookie-min.js", +] %] + [%# We should be able to set the default value of the header variable # to the value of the title variable using the DEFAULT directive, @@ -87,118 +95,33 @@ [% PROCESS 'global/setting-descs.none.tmpl' %] - [%# Set up the skin CSS cascade: - # 0. YUI CSS - # 1. Standard Bugzilla stylesheet set (persistent) - # 2. Standard Bugzilla stylesheet set (selectable) - # 3. All third-party "skin" stylesheet sets (selectable) - # 4. Page-specific styles - # 5. Custom Bugzilla stylesheet set (persistent) - # "Selectable" skin file sets may be either preferred or alternate. - # Exactly one is preferred, determined by the "skin" user preference. - #%] - [% IF user.settings.skin.value != 'standard' %] - [% user_skin = user.settings.skin.value %] - [% END %] - [% style_urls.unshift('skins/standard/global.css') %] - - [%# YUI dependency resolution %] - [%# We have to do this in a separate array, because modifying the - # existing array by unshift'ing dependencies confuses FOREACH. - #%] - [% SET yui_resolved = [] %] - [% FOREACH yui_name = yui %] - [% FOREACH yui_dep = yui_deps.${yui_name}.reverse %] - [% yui_resolved.push(yui_dep) IF NOT yui_resolved.contains(yui_dep) %] - [% END %] - [% yui_resolved.push(yui_name) IF NOT yui_resolved.contains(yui_name) %] - [% END %] - [% SET yui = yui_resolved %] - - [%# YUI CSS %] - [% FOREACH yui_name = yui %] - [% IF yui_css.$yui_name %] - - [% END %] - [% END %] + [% SET yui = yui_resolve_deps(yui, yui_deps) %] + [% SET css_sets = css_files(style_urls, yui, yui_css) %] [%# CSS cascade, part 1: Standard Bugzilla stylesheet set (persistent). # Always present. #%] - [% FOREACH style_url = style_urls %] - + [%# This allows people to switch back to the "Classic" skin if they + # are in another skin. + #%] + + [% FOREACH style_url = css_sets.standard %] + [% PROCESS format_css_link css_set_name = 'standard' %] [% END %] - - [%# CSS cascade, part 2: Standard Bugzilla stylesheet set (selectable) - # Present if skin selection is enabled. + [%# CSS cascade, part 2 & 3: Third-party stylesheet set (selected and + # selectable). All third-party skins are present as alternate + # stylesheets, even if they are not currently in use. #%] - [% IF user.settings.skin.is_enabled %] - [% FOREACH style_url = style_urls %] - - [% END %] - + [% FOREACH style_url = css_sets.skin %] + [% PROCESS format_css_link css_set_name = user.settings.skin.value %] [% END %] - [%# CSS cascade, part 3: Third-party stylesheet set (selectable). - # All third-party skins are present if skin selection is enabled. - # The admin-selected skin is always present. - #%] - [% FOREACH contrib_skin = user.settings.skin.legal_values %] - [% NEXT IF contrib_skin == 'standard' %] - [% NEXT UNLESS contrib_skin == user_skin - OR user.settings.skin.is_enabled %] - [% contrib_skin = contrib_skin FILTER url_quote %] - [% IF contrib_skin.match('\.css$') %] - [%# 1st skin variant: single-file stylesheet %] - - [% ELSE %] - [%# 2nd skin variant: stylesheet set %] - [% FOREACH style_url = style_urls %] - [% IF style_url.match('^skins/standard/') %] - - [% END %] - [% END %] - + [% FOREACH alternate_skin = css_sets.alternate.keys %] + [% FOREACH style_url = css_sets.alternate.$alternate_skin %] + [% PROCESS format_css_link css_set_name = alternate_skin %] [% END %] [% END %] @@ -214,33 +137,19 @@ # Always present. Site administrators may override all other style # definitions, including skins, using custom stylesheets. #%] - [% FOREACH style_url = style_urls %] - [% IF style_url.match('^skins/standard/') %] - - [% END %] + [% FOREACH style_url = css_sets.custom %] + [% PROCESS format_css_link css_set_name = 'standard' %] [% END %] - [%# YUI Scripts %] - - [% FOREACH yui_name = yui %] - + [% starting_js_urls.push("js/yui/$yui_name/${yui_name}.js") %] [% END %] + [% starting_js_urls.push('js/global.js') %] - + [% FOREACH javascript_url = starting_js_urls %] + [% PROCESS format_js_link %] + [% END %] - [% IF javascript_urls %] - [% FOREACH javascript_url = javascript_urls %] - - [% END %] + [% FOREACH javascript_url = javascript_urls %] + [% PROCESS format_js_link %] [% END %] [%# this puts the live bookmark up on firefox for the Atom feed %] @@ -380,3 +287,41 @@ [% IF message %]
[% message %]
[% END %] + +[% BLOCK format_css_link %] + [% IF style_url.match('/IE-fixes\.css') %] + ' IF style_url.match('/IE-fixes\.css') %] +[% END %] + +[% BLOCK format_js_link %] + +[% END %] diff --git a/template/en/default/setup/strings.txt.pl b/template/en/default/setup/strings.txt.pl index 7e590cb3e..62b645672 100644 --- a/template/en/default/setup/strings.txt.pl +++ b/template/en/default/setup/strings.txt.pl @@ -71,6 +71,8 @@ END feature_updates => 'Automatic Update Notifications', feature_xmlrpc => 'XML-RPC Interface', + file_remove => 'Removing ##name##...', + file_rename => 'Renaming ##from## to ##to##...', header => "* This is Bugzilla ##bz_ver## on perl ##perl_ver##\n" . "* Running on ##os_name## ##os_ver##", install_all => <