Closed
Bug 126955
(bz-l10n)
Opened 23 years ago
Closed 22 years ago
Bugzilla should support translated/localized templates
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: burnus, Assigned: burnus)
References
Details
Attachments
(1 file, 17 obsolete files)
(deleted),
patch
|
gerv
:
review+
|
Details | Diff | Splinter Review |
Bugzilla should support a translated user interface. The first and easierst step is to translate the templates which is almost enough for the beginning since those contain most of the stings to be translated. (There are two kinds of installations: Those which use only one language and those which support bugreports in several languages) Proposed new directory layout (inspired by /usr/share/man/): templatex/default/ -- default language (en) templatex/default/<languagecode>/ -- other languages (such as de, en-GB,...) templatex/custom/ -- default language (en) templatex/custom/<languagecode>/ -- other languages The cgi's should then support the Accept-Language header to choose the template (this is not the easiest task since the Accept-Language header can contain things like fr-FR which should match fr-FR and fr ...) I have a "default/de/" ready sitting here, if someone wants to check it in, drop a mail. (This is actually independend of having the neede perl support since one can us it to replace the English version in default. And my have the time to dig into the perl part. checksetup.pl should probably used to create a list of supported languages and the admin should probably have the possibility to restrict the languages to a subset (this sounds a bit like hacking defparms.pl). Additionally a link list where one can choose the desired language would be neat (such as %commandmenu%).
Comment 1•23 years ago
|
||
I'm interested in the "/default/de/" template-set. I've set up a bugzilla-installation under http://www.felliweb.de (click on Bugzilla). I intend it to be used as a demonstration system for the german translation of bugzilla. I also talked with KaiRo about translating bugzilla, and he seemed interested. If we are going to translate Bugzilla into other languages, the use of non-ASCII characters in bug reports must not lead to invalid mail messages (Bug #110692)
Comment 2•23 years ago
|
||
There are some major issues here related to directory structure within the bugzilla hierarchy in order to facilitate this, which need to be resolved before 2.16 is released, whether or not any localized templates are actually included with 2.16. I will post a message to get some discussion of this going on news:netscape.public.mozilla.webtools (mozilla-webtools mailing list), as it needs a broader audience and participation than the people watching this bug.
Comment 3•23 years ago
|
||
per my last comment, moving this to 2.16 so we don't forget to deal with the directory structure before release.
Target Milestone: --- → Bugzilla 2.16
Comment 4•23 years ago
|
||
checksetup.pl contains strings that should be localized ("This bug has been marked as a duplicate..."), because they are written to the db. Can we use templates here too? Error and diagnostic messages should probably left as they are.
Comment 5•23 years ago
|
||
bug 135707 is going to satisfy the 2.16 requirements for this. Moving this bug out to get it off the 2.16 list.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Assignee | ||
Comment 6•23 years ago
|
||
This is the first version of a patch which allows multiple languages based on the HTTP_ACCEPT_LANGUAGE header as per discussion on news://netscape.public.mozilla.webtools. The supported languages can be set in editparms.cgi. If no suitable HTTP_ACCEPT_LANGUAGE is found, the first language is used. Note that I include the all languages from the list (editparms.cgi) since I want to honour the case that not all templates have been translated.
Assignee | ||
Updated•23 years ago
|
Assignee | ||
Comment 7•23 years ago
|
||
Updated•23 years ago
|
Attachment #86617 -
Flags: review-
Comment 8•23 years ago
|
||
Comment on attachment 86617 [details] [diff] [review] Second version: Now searches custom, default instead of twice custom ;-) Oops, sorry about that - wrong button. >+sub TemplateIncludePath () { >+ my $acceptheader = $ENV{HTTP_ACCEPT_LANGUAGE} || ""; >+ my $availlangs = Param("languages"); Normally, we name the variable we copy a param into after that param. >+ $acceptheader =~ s/;q=0-9//g; # remove language priority and assume that they are ordered This won't remove all q values, which can have a decimal point. You want [0-9\.]+ or something like it. >+ $acceptheader =~ s/[^A-Z_a-z,]//g; # taint >+ $availlangs =~ s/[^A-Z_a-z,]//g; Are language and sub-language parts not ever hyphen-separated? >+ # Try one: Match exactly: 'en_US' matches 'en_US', 'en' matches 'en' and 'en_US' etc. >+ foreach(split /,/, $acceptheader) { >+ if ( $availlangs =~ m/[^\w]($_\w*)/ ) { If [^\w] means non-word char, use \W. And this will fail if it's first in the list. You want: my @acceptlangs = split(',', $acceptheader); if grep($_, @acceptlangs) { $usedlang = $_; last; } and then if grep("$__\w+", @acceptlangs) { Basically, the aim of this function is to create a list of languages to try, where the list is the intersection of ACCEPT_LANGUAGE and the Param, with the default appended to the end (it may appear twice; that's not a problem.) You then create the template search path from this list using map() and join(":", ...). >+DefParam("languages", >+ "A comma-separted list of <a href=\"http://lcweb.loc.gov/standards/iso639-2/englangn.html\">ISO 639 language codes</a> of the languages in which Bugzilla output may be displayed. The ACCEPT_LANGUAGE header is used to determin the actually used language, if no suitable ACCEPT_LANGUAGE is available, the first language is used.", "A comma-separated list of <a href=\"http://lcweb.loc.gov/standards/iso639-2/englangn.html\">ISO 639 language codes</a> for the languages in which you wish Bugzilla output to be displayed. Note that you must install the appropriate language packs before adding languages to this Param. The language used is the first language in this list which is present in the ACCEPT_LANGUAGE header which the user's browser sends. If there are no matches, Bugzilla falls back on the first language in this list." Gerv
Assignee | ||
Comment 9•23 years ago
|
||
Gerv wrote: > Normally, we name the variable we copy a param into after that param. fixed. > Are language and sub-language parts not ever hyphen-separated? I haven't found a RFC instancely, but Mozilla uses en-us whereas under UNIX locale there is en_US. (I tend to mix them; thus I match both of them.) > if grep($_, @acceptlangs) { This will never work. As `man perlfunc` says: "`$_' is a reference into the list value" and thus you cannot use foreach(@foo) {grep(/$_/,... and then if grep("$__\w+", @acceptlangs) { > You then create the template search path from this list using map() and > join(":", ...). done. >+DefParam("languages", Used this DefParam explanation.
Assignee | ||
Comment 10•23 years ago
|
||
Updated•23 years ago
|
Attachment #86927 -
Flags: review-
Comment 11•23 years ago
|
||
Comment on attachment 86927 [details] [diff] [review] Fourth version: Fix 'use strict' error (add twice a 'my') >+sub TemplateIncludePath () { Nit: call it GetTemplateIncludePath(). >+ my $languages = Param("languages"); >+ $languages =~ s/[^A-Z_a-z,-]//g; >+ my @languages = split (',', $languages); >+ >+ my $accept_language = $ENV{HTTP_ACCEPT_LANGUAGE} || ""; >+ # remove language priority and assume that they are ordered >+ $accept_language =~ s/;q=[0-9\.]+//g; >+ # taint >+ $accept_language =~ s/[^A-Z_a-z,-]//g; >+ my @accept_language = split(',', $accept_language); Rearrange as above for clarity, and rename @accept_language to @accept_languages. >+ my $usedlang = ""; >+ >+ # Try one: Match exactly: 'en-us' matches 'en-us', 'en' matches 'en' and 'en-us' etc. >+ foreach(@accept_language) { >+ my $al = $_; >+ if(my @found = grep(/$al/i, @languages)) { >+ $usedlang = $found[0]; >+ last; >+ } >+ } >+ >+ # Try two: en-us matches 'en' >+ if( $usedlang eq '') { >+ foreach(@accept_language) { >+ my $al = $_; >+ $al =~ s/[-_].*//; >+ if(my @found = grep(/$al/i, @languages)) { >+ $usedlang = $found[0]; >+ last; >+ } >+ } >+ } >+ >+ my $includepath = join(':', map("template/$_/custom:template/$_/default",@languages)); >+ >+ if($usedlang ne '') { >+ $includepath = "template/$usedlang/custom:template/$usedlang/default:$includepath"; >+ } >+ return $includepath; I don't think this quite does the right thing. It creates an includepath consisting of a single language from accept_languages, plus the entire language list. What you want is an include path consisting of: 1) all the languages both in ACCEPT_LANGUAGES and the Param, in the order they appear in ACCEPT_LANGUAGES (except more specific ones first). 2) the default language from the Param (which should be guaranteed to have every template.) This makes it as likely as possible that the user will be given a template with a language they understand. This is how it should work. I really think the default language should be its own param - anything else is counter-intuitive. At the front of the list is wrong, because it's the last place to look. At the back of the list is wrong, because it implies they are searched in the order they are in the Param, which they are not (they are searched in ACCEPT_LANGUAGE order.) So, Param('languages') is an _unordered_ list. Say you have Params as follows: Param('defaultlanguage') = "en"; Param('languages') = "de-AT, fr, en"; What should the following accept-headers give as search paths? ACCEPT1: "de"; This person should get "en". This is because by calling the localisation "de-AT", the admin is saying it is not suitable for other Germans speakers apart from those in Austria. If it was suitable for all, it would be called "de". This is a key point. ACCEPT2: "de-AT, en"; This person should get "de-AT", and then "en". ACCEPT3: "fr, de-AT"; This person should get "fr" then "de-AT", then "en", because order in ACCEPT takes precedence over order in the Param. So, how to implement this? The algorithm is as follows (in pseudocode): foreach my $accept_language (@accept_languages) { grep($accept_language, @param); if (match) { push(match, @include_languages); } $accept_language =~ s/([-_].*)//; if ($1) { grep($accept_language, @param); if (match) { push(match, @include_languages); } } } push(default, @include_languages); join, map etc. Gerv
Assignee | ||
Comment 12•23 years ago
|
||
> What should the following accept-headers give as search paths?
> This person should get "en". This is because by calling the
> localisation"de-AT", the admin is saying it is not suitable for other Germans
> speakers apart from those in Austria. If it was suitable for all, it would be
> called "de". This is a key point.
That way ok, but I strongly believe a HTTP_ACCEPT_LANGUAGE header with de-at
should also match to languages 'de'. Usually this doesn't mean that they are
opposed to 'de' but that they favour 'de-at'. (Yes they should then use
'de-at,de' but they ususally don't to.) More over as a simple user I wont be
able to specify all that languages that there are available:
en,en-us,en-uk,en-uk-southwestlondon, en-uk-myowndialect etc.
Assignee | ||
Comment 13•23 years ago
|
||
I now actually use the ;q=... values of the Accept-Language header (RFC 2616, I finally found the standard). I reworked additionally the logic according to both the standard and to Gerv (therefore I also introduced Param('defaultlanguage'). Additionally the code is -- in my humble opinion -- much cleaner, shorter and more beautiful.
Assignee | ||
Comment 14•23 years ago
|
||
Gerv wrote and I included it also in the latest patch: > The language used is the first language in this list which is present in the > ACCEPT_LANGUAGE header which the user's browser sends. This is actually not true (and never was). It is rather the otherway round: The first language (with the highst q-value or which comes first (same q-value)) which matches the Param(languages) is used (and all other matching languages are also added to the path). Can someone formulate this in nice plain English? I found another nit in my code: >+ foreach(split /,/, $accept_language) { >+ my ($lang, $qvalue) = split /;q=/; next if not defined $lang; is missing (to prevent a warning if a '*' is present, which is valid according RFC 2616).
Assignee | ||
Updated•23 years ago
|
Alias: bz-l10n
Comment 15•23 years ago
|
||
I, unable to do a correct query did not find these patches and wrote my own, available with bug 152556. However, bug 152556 takes the question one step further: Loads of content are generated directly from the perl source and are thus not influenced by the templates. One example are the titles for the bug list. Another one are the bug stati ('ASSIGNED' never shows up in any template). Using templates for each of these sureley would be possible, but IMHO unpractical. We should be comming up with some translation code that gets the english word or short phrase thrown upon and then comes up with a translation. Translations could then be read from flat files, one per language.
Comment 16•23 years ago
|
||
*** Bug 152556 has been marked as a duplicate of this bug. ***
Comment 17•23 years ago
|
||
Thomas: we have plans for how to localise the rest of the Bugzilla interface, probably using gettext(). If you have ideas, post them to the newsgroups. Gerv
Comment 18•23 years ago
|
||
Comment on attachment 86948 [details] [diff] [review] Fifth version: Complete rework >+ my @accept_language = sortAcceptLanguage($ENV{HTTP_ACCEPT_LANGUAGE} || ""); AFAIAA, Bugzilla style is to use quotes on hash keys. You do this in a couple of places. >+DefParam("languages", >+ "A comma-separated list of RFC 1766 language tags of languages in which you wish Bugzilla output to be displayed. Note that you must install the appropriate language packs before adding languages to this Param. The language used is the first language in this list which is present in the Accept-Language header which the user's browser sends.", "A comma-separated list of RFC 1766 language tags. These identify the languages in which you wish Bugzilla output to be displayed. Note that you must install the appropriate language pack before adding a language to this Param. The language used is the one in this list with the highest q-value in the user's Accept-Language header." >+DefParam("defaultlanguage", >+ "The language Bugzilla falls back on if no matching Accept-Language header can be found.", "The UI language Bugzilla falls back on if no suitable language is found in the user's Accept-Language header." Other than those nits, this stuff is perfect :-) Gerv
Attachment #86948 -
Flags: review+
Assignee | ||
Comment 19•23 years ago
|
||
Attachment #86613 -
Attachment is obsolete: true
Attachment #86617 -
Attachment is obsolete: true
Attachment #86926 -
Attachment is obsolete: true
Attachment #86927 -
Attachment is obsolete: true
Attachment #86948 -
Attachment is obsolete: true
Assignee | ||
Comment 20•23 years ago
|
||
Attachment #88318 -
Attachment is obsolete: true
Updated•23 years ago
|
Attachment #88320 -
Flags: review+
Comment 21•23 years ago
|
||
Comment on attachment 88320 [details] [diff] [review] Seventh version: Fix '*' problem mentioned in comment 14 >+# Make an ordered list out of a HTTP Accept-Language header see RFC 2616, 14.4 >+# We ignore '*' and <language-range>;q=0 >+# For languages with the same priority q the order remains unchanged. >+sub sortAcceptLanguage ($) { >+ sub sortQvalue { $b->{'qvalue'} <=> $a->{'qvalue'} } Bugzilla standard is a 4-space indent, and we don't use function prototypes. >+ # clean up string. >+ $accept_language =~ s/[^A-Za-z;q=0-9\.\-,]//g; Isn't using "tr" more efficient here? I don't know... >+ push(@qlanguages,{ 'qvalue' => $qvalue, 'language' => $lang}); Nit: push(@qlanguages, {'qvalue' => $qvalue, 'language' => $lang}); >+ push(@usedlanguages,Param('defaultlanguage')); Nit: push(@usedlanguages, Param('defaultlanguage')); Gerv
Assignee | ||
Comment 22•23 years ago
|
||
Gerv wrote: >(From update of attachment 88320 [details] [diff] [review]) > >+sub sortAcceptLanguage ($) { > >+ sub sortQvalue { $b->{'qvalue'} <=> $a->{'qvalue'} } > Bugzilla standard is a 4-space indent, and we don't use function prototypes. Fixed spacing and removed '($)'. > >+ $accept_language =~ s/[^A-Za-z;q=0-9\.\-,]//g; > Isn't using "tr" more efficient here? I don't know... I failed to get tr working (delete everything except ..., for me only delete nothing except ... worked), but I'm not a PERL specialist. >Nit: push(@qlanguages, {'qvalue' => $qvalue, 'language' => $lang}); >Nit: push(@usedlanguages, Param('defaultlanguage')); Fixed those and a few more.
Assignee | ||
Updated•23 years ago
|
Attachment #88320 -
Attachment is obsolete: true
Comment 23•23 years ago
|
||
Comment on attachment 88445 [details] [diff] [review] Eighth version: Whitespace fixes, remove ($) prototyping r=gerv. Gerv
Attachment #88445 -
Flags: review+
Comment 24•22 years ago
|
||
Fixed. Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.185; previous revision: 1.184 done Checking in defparams.pl; /cvsroot/mozilla/webtools/bugzilla/defparams.pl,v <-- defparams.pl new revision: 1.82; previous revision: 1.81 done Gerv
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 25•22 years ago
|
||
This seems to have caused a regression. If a new installation runs checksetup with a fresh-checkout where the data directory has not even been created, checksetup requires globals, globals now checks a Param, (languages), which causes it to realize that Params aren't loaded. When defparams runs, it chokes if the data directory doesn't yet exist.
Severity: normal → critical
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 26•22 years ago
|
||
'joy'. We need to up the requirement to TT 2.08, and then pass a sub ref in as the INCLUDE_PATH param, which will remove those issues. You also totally ignored the directive in globals.pl to update checksetup.pl at t/004template.t with the new params.... Those two need to end up searching _all_ existing template paths, though, so you'll need to change the File::Find calls, too.
Comment 27•22 years ago
|
||
I'm going to back out the existing patch at 9pm PDT (-0700) tonight unless we have a corrective patch to fix it by then, in the interest of keeping the cvs tip at least usable. You have T minus 2 hours, 3 minutes, and counting.
Comment 28•22 years ago
|
||
Backed out. tcsh> cvs update -j1.82 -j1.81 defparams.pl RCS file: /cvsroot/mozilla/webtools/bugzilla/defparams.pl,v retrieving revision 1.82 Merging differences between 1.82 and 1.81 into defparams.pl tcsh> cvs update -j1.185 -j1.184 globals.pl RCS file: /cvsroot/mozilla/webtools/bugzilla/globals.pl,v retrieving revision 1.185 retrieving revision 1.184 Merging differences between 1.185 and 1.184 into globals.pl tcsh> cvs commit Checking in defparams.pl; /cvsroot/mozilla/webtools/bugzilla/defparams.pl,v <-- defparams.pl new revision: 1.85; previous revision: 1.84 done Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.191; previous revision: 1.190 done
Comment 29•22 years ago
|
||
I'll revisit this next week. bbaetz: what are "those two"? Gerv
Comment 30•22 years ago
|
||
justdave, myk, bbaetz: bbaetz says the easiest solution here is to up the TT required version to 2.08. Is that feasible? Gerv
Comment 31•22 years ago
|
||
Yeah, that should work. Pass a sub ref in within an array ref. see perldoc Template::Provider. Note that this subref will be called for every template/INCLUDE/PROCESS, so it should be fast. What you should do is compute an array of available template params (and cache it), then for each entry in the accept lang stuff, check for an exact match, or a prefix match (but see below). You can also (as of 2.08) return an arrayref of entries, rather than a string. However, I'm wondering how this will work with non-CGI templates. Also, the patch is wrong - en-gb does not match against en, although en does match against en-gb. See the last paragraph of rfc2616 section 14 (theres a mozilla bug somewhere to improve the ui) As well, order matters - if en is before en-GB, then the generic en templates should be used even if en-GB exists.
Assignee | ||
Comment 32•22 years ago
|
||
> Also, the patch is wrong - en-gb does not match against en, although en does > match against en-gb. # @accept_language = HTTP_ACCEPT_LANGUAGE; @languages = Param('languages') + foreach my $lang (@accept_language) { + # match exactly (case insensitive) + if(my @found = grep /^$lang$/i, @languages) { + push (@usedlanguages, $found[0]); + } This should match 'en-gb' as 'en-gb', shouldn't it? And 'en' should only match 'en' and not 'en-gb'. Or do I miss something obvious? > As well, order matters - if en is before en-GB, then the generic en templates > should be used even if en-GB exists. I believe this is the case (with the exception that the q-values take precedence) since I check for every HTTP_LANGUAGE_HEADER whether this language is present in the Param('languages') and push it if it is the case. Then I remove the '-.*' and push it also - if needed. The only thing that I am aware of which doesn't match RFC 2616 is that 'en-gb-cockney' doesn't look also for 'en-gb' but only for 'en'. For the order, there may be a problem buried: The sort algorithm may change the order for items with the same (or no) q value depending on the algorithm used by PERL. (This shouldn't happen with Mozilla since it passed the q-values and with @language = 'de','en' it works also well with Netscape 4.x -- even it I use @HTTP_ACCEPT_LANGUAGE = 'de-de', 'en'.)
Comment 33•22 years ago
|
||
en in ACCEPT_LANGUAGES is a valid match for an en-GB template, but not the other way arround. Also, for the template lang foo-bar-baz, you need to test if foo-bar and foo match those (Which they do, per the RFC)
Comment 34•22 years ago
|
||
> Also, for the template lang foo-bar-baz, you need to test if foo-bar and foo
> match those (Which they do, per the RFC)
Given that this will complicate the code, and the fact that the vast majority of
codes are two-part, I think that this enhancement can be made when the first
localisation which actually uses a three-or-more-part language code is proposed :-)
Gerv
Assignee | ||
Comment 35•22 years ago
|
||
I "rediffed" the eight patch due to the changes in defparams.pl. This version checks wether data/params exists and if not returns a default path. The generated path is now also used in ./checksetup.pl. This does't include changes to t/004template.t (yet). And I don't use the TT 2.08 feature to pass a sub to INCLUDE_PATH (which would have the advantage that there is not yet another location where data/params). The main reason for this patch is to provide a working solution (TM) for users of e.g. http://bugzilla-de.sf.net/.
Attachment #88445 -
Attachment is obsolete: true
Comment 36•22 years ago
|
||
Comment on attachment 101093 [details] [diff] [review] Nineth version: Rediff due to changes, check -e data/params Yeah, but you can't do that. The problem is that you need to precompile _all_ the templates - ie checksetup and the tests need to traverse each of template/*/{custom,default}, otherwise you get permission problems when apache writes stuff, and so on.
Attachment #101093 -
Flags: review-
Assignee | ||
Comment 37•22 years ago
|
||
I really failed to get this work with TT 2.08, it works with TT 2.08a though. Maybe this is only due to my broken setting or the broken program code (Debugging is really broken in TT 2.08 thus I needed TT 2.08a to fix my code). Left to do as an exercise: The t/* stuff.
Attachment #101093 -
Attachment is obsolete: true
Assignee | ||
Comment 38•22 years ago
|
||
"Internal" Testing result: It works with both TT 2.08 and TT 2.08a, i.e. I've only needed TT 2.08a for debuging purpose (debugging is broken in TT 2.08). I need to fix this though (TT 2.08/2.08a): doeditparams.cgi shows the following, if I edit defaultlanguage or languages: -------------------------------------------------------- Saving new parameters: Changed defaultlanguage We don't have shadow databases turned on; no syncing performed. OK, done. Edit the params some more. Go back to the query page. Bugzilla has suffered an internal error. Please save this page and send it to ZEDV Bugzilla Maintainer with details of what you were doing at the time this message appeared. URL: http://bugzilla.xxxx.yyyy/doeditparams.cgi Template->process() failed twice. First error: Insecure dependency in rename while running with -T switch at /usr/lib/perl5/Template/Document.pm line 286. Second error: Insecure dependency in rename while running with -T switch at /usr/lib/perl5/Template/Document.pm line 286. ---------------------------- Since everything is saved, it works, but it doesn't look very nice. Any ideas on how to fix this are welcome.
Assignee | ||
Comment 39•22 years ago
|
||
The "Insecure dependency in rename while running with -T switch at /usr/lib/perl5/Template/Document.pm line 286." was due to the fact that I didn't cache the template include path during debugging (see the '#') + return #$::vars->{'template-includepath'} = + [split /:/, join(':', map("template/$_/custom:template/$_/default", @usedlanguages))]; Fixed now. I'd like to see how this is handled when used as mod_perl... Please review.
Attachment #102084 -
Attachment is obsolete: true
Comment 40•22 years ago
|
||
Comment on attachment 102583 [details] [diff] [review] Eleventh version: Fix caching of the template include path This looks excellent. We can check it in as soon as we've coordinated the upgrade to TT 2.08 with the developers. I have one other query - would it be worth implementing an optimisation for when Param{'languages'} is a single value, which will be the common case (usually "en")? + if ($::vars->{'template-includepath'}) { + return $::vars->{'template-includepath'}; + } my $languages = Param{'languages'}; if (length($languages) == 2) { return "template/$languages/custom:template/$languages/default"; } Gerv
Attachment #102583 -
Flags: review+
Comment 41•22 years ago
|
||
Comment on attachment 102583 [details] [diff] [review] Eleventh version: Fix caching of the template include path >+sub getTemplateIncludePath () { >+ # Return cached value if available >+ if ($::vars->{'template-includepath'}) { >+ return $::vars->{'template-includepath'}; >+ } This doesn't belong in $::vars - use some other global instead >+ my @languages = (); >+ { >+ use File::Find; >+ sub languageDirectories { >+ my ($dev,$ino,$mode,$nlink,$uid,$gid); >+ $File::Find::name =~ m/^[^\/]*\/([a-z-]*)$/i && $_ ne 'CVS' >+ && (($dev,$ino,$mode,$nlink,$uid,$gid) = lstat($_)) && -d _ >+ && push(@languages, $_); >+ } >+ find({wanted => \&languageDirectories, bydepth => 1}, 'template'); >+ } Why do you need lstat? what is that checking for? > my $redir = ($^O =~ /MSWin32/i) ? "NUL" : "/dev/null"; > my $template = Template->new( > { >@@ -928,7 +940,7 @@ > OUTPUT => $redir, > > # Colon-separated list of directories containing templates. >- INCLUDE_PATH => "template/en/custom:template/en/default", >+ INCLUDE_PATH => join(':', map("template/$_/custom:template/$_/default",@languages)) , > This won't work, because this will only then find stuff form the first language, not all teh languages. You need to do this one language at a time, for all supported languages, with the appropriate INCLUDE_PATH.
Attachment #102583 -
Flags: review-
Assignee | ||
Comment 42•22 years ago
|
||
Gerv wrote: > I have one other query - would it be worth implementing an optimisation for > when Param{'languages'} is a single value, which will be the common case > (usually "en")? included. Bradley Baetz wrote: > This doesn't belong in $::vars - use some other global instead fixed Additionally I changed the compilation of the templates in checksetup. This is based on the attachment 102666 [details] [diff] [review] of bug 168191 and basically adds a foreach and moves the code a bit around. I still haven't updated t/004template.t and t/Support/Templates.pm yet
Attachment #102583 -
Attachment is obsolete: true
Comment 43•22 years ago
|
||
Comment on attachment 102670 [details] [diff] [review] Twelfth version: fix checksetup compilation, other review fixed This looks fine. You can drop teh definition of $redir in checksetup (since its no longer used). Update the other files, and I'll r= this
Attachment #102670 -
Flags: review-
Comment 44•22 years ago
|
||
ping?
Comment 45•22 years ago
|
||
Yeah, we need to get this in... we already have two localizations other than English available, and we need to stop forcing them to say "change this line in globals.pl to use these".
Assignee | ||
Comment 46•22 years ago
|
||
- Rediff due to the checkin of bug 168191 - Finally updated t/* (Plus fix warning in t/007util.t) Sorry that it took that long, but I had to learn for an exam. (This breakes the current patch to bug 173622, but I don't care and a rediff is easily done ;-)
Attachment #102670 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #107031 -
Flags: review?(bbaetz)
Comment 47•22 years ago
|
||
Comment on attachment 107031 [details] [diff] [review] Thirteenth version: Rediff + modification to the t/* tests >Index: checksetup.pl >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v >retrieving revision 1.204 >diff -u -r1.204 checksetup.pl >--- checksetup.pl 19 Nov 2002 07:19:30 -0000 1.204 >+++ checksetup.pl 21 Nov 2002 17:02:00 -0000 >@@ -1,4 +1,4 @@ >-#!/usr/bonsaitools/bin/perl -w >+#!/usr/bin/perl -w > # -*- Mode: perl; indent-tabs-mode: nil -*- > # > # The contents of this file are subject to the Mozilla Public >@@ -25,6 +25,7 @@ > # Zach Lipton <zach@zachlipton.com> > # Jacob Steenhagen <jake@bugzilla.org> > # Bradley Baetz <bbaetz@student.usyd.edu.au> >+# Tobias Burnus <burnus@net-b.de> > # > # > # Direct any questions on this source code to >@@ -944,33 +945,26 @@ > finddepth(\&remove, 'data/template'); > } > >+ # Search for template directories >+ # We include the default and custom directories separately to make >+ # sure we compile all templates >+ my @templatepaths = (); >+ { >+ use File::Find; >+ sub languageDirectories { >+ my ($dev,$ino,$mode,$nlink,$uid,$gid); >+ if( $File::Find::name =~ m/^[^\/]*\/([a-z-]*)$/i Nit: standard formatting is if ($File::... >+ foreach $::templatepath(@templatepaths) { Again, a nit: Space after $::templatepath. >+ $::provider = Template::Provider->new( >+ { And line the opening bracket up with the $ sign. > # Colon-separated list of directories containing templates. >- INCLUDE_PATH => "template/en/custom:template/en/default" , >+ INCLUDE_PATH => [\&getTemplateIncludePath], Remind me: does this require a bump in the required version of TT? > + my $languages = Param('languages'); > + if (length($languages) == 2) { > + return $::template_include_path = > + ["template/$languages/custom", "template/$languages/default"]; > + } Would it not be better to check for the existence of a comma, instead of the length? (A comma would indicate multiple values). Also, trim() the languages Param when you read it, to avoid nasty problems with this shortcut. Gerv
Comment 48•22 years ago
|
||
Comment on attachment 107031 [details] [diff] [review] Thirteenth version: Rediff + modification to the t/* tests Don't change teh #! line + + eval("use Template"); + my $redir = ($^O =~ /MSWin32/i) ? "NUL" : "/dev/null"; You don't need $redir + # Per RFC 1766 and RFC 2616 any language tag matches also its + # primary tag. That is en-gb matches also en but not en-uk + $lang =~ s/(-.*)//; + if($1 ne '') { Just test $1 - if its not there, $1 will be undef, and there will be warnings comparing it to ''. + [split /:/, join(':', map("template/$_/custom:template/$_/default", + @usedlanguages))]; Why are you splitting and then joining? Also, this may be better to do after my patch - instead of caching the include path in a $:: global var, you can calculate it whn the user joins, in hte Bugzilla object. IF this gets in before that, though, then I guess I'll do it. You also do need to bump the requried TT version. Also, zach or someoe else familiar with the tests shold look at those changes.
Attachment #107031 -
Flags: review?(bbaetz) → review-
Assignee | ||
Comment 49•22 years ago
|
||
> Remind me: does this require a bump in the required version of TT? It does. Somehow it got lost (it was present in tenth version). > Would it not be better to check for the existence of a comma, instead of the > length? Did so now. > You don't need $redir Now I finally delete it (should have done so after comment 43). > Don't change teh #! line Good catch! This is the disadvantage of using a real world installation for testing. > Why are you splitting and then joining? Because I wasn't aware of the fact that map accepts a list as first parameter (and didn't try whether it works - it does). This additionally fixes bug 162224 (t/006spellcheck.....Use of uninitialized value). [006spellcheck uses tons of tabs, maybe t/005no_tabs.t should also check the t/* files ...]
Attachment #107031 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #107132 -
Flags: review?(bbaetz)
Comment 50•22 years ago
|
||
Comment on attachment 107132 [details] [diff] [review] Fourteenth version: Fix review nits. >Index: checksetup.pl >+ my @templatepaths = (); >+ { >+ use File::Find; >+ sub languageDirectories { >+ my ($dev,$ino,$mode,$nlink,$uid,$gid); What are those for? >+ if($File::Find::name =~ m/^[^\/]*\/([a-z-]*)$/i >+ && $_ ne 'CVS' && -d $_) { You don't need the bracketing. Also, will this work with windows, where it uses \ ? Rather than using File::Find, aren't you better off doing readdir for 'template', and then using File::Spec to join the paths for custom/default? > return if ($name =~ /\/CVS\//); > return if ($name !~ /\.tmpl$/); >- $name =~ s!template/en/default/!!; # trim the bit we don't pass to TT >+ $name =~ s!$::templatepath/!!; # trim the bit we don't pass to TT s/\Q$::templatepath\E// in case the lang code contains a ., or something. > > chdir($::baseDir); > > # Do this to avoid actually processing the templates >- my ($data, $err) = $provider->fetch($name); >+ my ($data, $err) = $::provider->fetch($name); If it makes it easier, you can also use no_chdir for File::Find now that require perl 5.6. >+ # Disable warnings which come from running the compiled templates >+ # This way is OK, because they're all runtime warnings. >+ # The reason we get these warnings here is that none of the required >+ # vars will be present. >+ local ($^W) = 0; Actually, you can drop this now that we no longer run the templates, just compile them >--- t/007util.t 10 Nov 2002 23:31:15 -0000 1.1 >+++ t/007util.t 22 Nov 2002 10:20:09 -0000 >@@ -63,7 +63,7 @@ > is(lsearch(\@list,'kiwi'),-1,'lsearch 3 (missing item)'); > > #max() and min(): >-my @list = (7,27,636,2); >+@list = (7,27,636,2); > is(max(@list),636,'max()'); > is(min(@list),2,'min()'); Why this change?
Attachment #107132 -
Flags: review?(bbaetz) → review-
Assignee | ||
Comment 51•22 years ago
|
||
> >+ my ($dev,$ino,$mode,$nlink,$uid,$gid); > What are those for? Leftovers from pfind (removed by rewrite of this section). > Rather than using File::Find, aren't you better off doing readdir for > 'template', and then using File::Spec to join the paths for custom/default? did so now. > s/\Q$::templatepath\E// in case the lang code contains a ., or something. Done. > If it makes it easier, you can also use no_chdir for File::Find now that > require perl 5.6. Did so. > Actually, you can drop this now that we no longer run the templates, just > compile them Removed those bits. > >-my @list = (7,27,636,2); > >+@list = (7,27,636,2); > Why this change? Well it is basically: my @list = ('apple','pear','plum','<"\\%'); -my @list = (7,27,636,2); +@list = (7,27,636,2); which fixes a warning.
Attachment #107132 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #107344 -
Flags: review?(bbaetz)
Comment 52•22 years ago
|
||
Comment on attachment 107344 [details] [diff] [review] Fiveteenth version: Further review fixes. This is fine, with a few minor changes. I do want zach (or someone) to review the tests stuff, though >Index: checksetup.pl >+ foreach my $dir (@files) { >+ next if($dir =~ /^CVS$/i); >+ my $path = File::Spec->catfile('template', $dir, 'custom'); This should be ->catdir, ratehr than catpath. Note that that way you get a trailing /, which may affect some of the below stuff >+ push(@templatepaths, $path) if(-d $path); >+ $path = File::Spec->catfile('template', $dir, 'default'); ditto >+ push(@templatepaths, $path) if(-d $path); >+ } >- $name =~ s!template/en/default/!!; # trim the bit we don't pass to TT >- >- chdir($::baseDir); >+ $name =~ s/\Q$::templatepath\/\E//; # trim the bit we don't pass to TT The trailing / should be outside the \Q\E. |perldoc perlre| refers to 'gory details' with backslashes inside \Q\E. Besides, \ isn't technically portable, although its accepted on all the OSs we support, I guess. >@@ -60,6 +61,7 @@ > $zz = $main::template; > $zz = $main::userid; > $zz = $main::vars; >+ $zz = $main::template_include_path This is ugly, but I'm rewriting that over in the other bug, so don't worry about it. >+ # clean up string. >+ $accept_language =~ s/[^A-Za-z;q=0-9\.\-,]//g; >+ my @qlanguages; >+ my @languages; >+ foreach(split /,/, $accept_language) { >+ my ($lang, $qvalue) = split /;q=/; Use a regexp rather than split, just in case someone does something silly with two qvalues. ie en_US;q=0;q=1 if (m/([A-Za-z\.\-]+)(?:;q=(\d+(?:\.\d+)))?/) { $lang = $1; $qval = $2 I think - do test, esp teh decimal point stuff (That regexp is meant to accept one or more of any letter, dot, or -, then optionally ;q=, some digits, and optionally .<some more digits>) That does the 'cleanup' there, too, and also more accurately validates the data. I've skiped the tests stuf; I'm not really familar with it.
Attachment #107344 -
Flags: review?(bbaetz) → review-
Comment 53•22 years ago
|
||
Actually, forget that regexp for qvalues - I changed my mind, and its not worth it. The rest still stands, though However, this is also wrong: + # Per RFC 1766 and RFC 2616 any language tag matches also its + # primary tag. That is en-gb matches also en but not en-uk + $lang =~ s/(-.*)//; + if($1) { + if(my @found = grep /^$lang$/i, @languages) { + push (@usedlanguages, $found[0]); + } + } a) You need to test the result of the s//, not $1 - if it doesn't match, $1 will be left at whatever it was last time it was used. b) Also, as I mentioned earlier (see comment #33), this is the wrong way arround. A _client_ (ie $lang) of en matches a _server_ value of en-US, not the other way arround.
Assignee | ||
Comment 54•22 years ago
|
||
> I do want zach (or someone) to review the tests stuff, though will do an request. > This should be ->catdir, ratehr than catpath Fixed this with all implications. > Use a regexp rather than split, just in case someone does something > silly with two qvalues. ie en_US;q=0;q=1 > if (m/([A-Za-z\.\-]+)(?:;q=(\d+(?:\.\d+)))?/) { > (That regexp is meant to accept one or more of any letter, dot, or -, then well the language may not contain a dot, and the number starts with one 0 or a 1 followed (optionally) by a dot, so I have now: if (m/([A-Za-z\-]+)(?:;q=(\d(?:\.\d+)))?/) { See http://www.ietf.org/rfc/rfc2616.txt, Sec. 14.4
Attachment #107344 -
Attachment is obsolete: true
Assignee | ||
Comment 55•22 years ago
|
||
Comment on attachment 107354 [details] [diff] [review] Sixteenth version: Fix the last nits of bbaetz. (request review) Zach, can you especially look at the changes of the tests (t/004template.t etc.)
Attachment #107354 -
Flags: review?(zach)
Comment 56•22 years ago
|
||
Comment on attachment 107354 [details] [diff] [review] Sixteenth version: Fix the last nits of bbaetz. You don't appear to have fixed teh language thing I mentioned in my followup comment
Assignee | ||
Comment 57•22 years ago
|
||
> You don't appear to have fixed teh language thing I mentioned in my followup > comment Since I didn't see it. (I attached without reloading the page first.) > a) You need to test the result of the s//, not $1 - if it doesn't match, $1 > will be left at whatever it was last time it was used. Going to fix this later. First I need to understand the following problem: > b) Also, as I mentioned earlier (see comment #33), this is the wrong way > arround. A _client_ (ie $lang) of en matches a _server_ value of en-US, not > the other way arround I still believe this is correct: ACCEPT MATCHES (http header) (available languages: e.g. en, en-gb) en en Exact match en-gb en-gb Exact match (en-us) (no exact match) en-us => en en Main language part matching And I believe this happens. It seems you want to do it the otherway round: Accept=en should match available=('en-us')? See http://www.ietf.org/rfc/rfc2616.txt, section 14.4: "A language-range matches a language-tag if it exactly equals the tag, or if it exactly equals a prefix of the tag such that the first tag character following the prefix is "-"."
Comment 58•22 years ago
|
||
Comment on attachment 107354 [details] [diff] [review] Sixteenth version: Fix the last nits of bbaetz. As long as you're gonna be uploading a new version anyway, here's my comments regarding the changes to the tests... >Index: t/004template.t >=================================================================== >@@ -54,68 +55,84 @@ > } > } > >-my $include_path = $Support::Templates::include_path; >+my @languages = @Support::Templates::languages; >+my @include_paths = @Support::Templates::include_paths; >+my %include_path = %Support::Templates::include_path; Instead of doing all this, you can export these variables from Support::Templates use base qw(Exporter); @Support::Files::EXPORT = qw(@languages @include_paths %include_path); >+sub existOnce { >+ foreach(@_) { >+ return $_ if -e $_; >+ } >+ return 0; >+} I have a tendeny to not like using $_ and @_ quite so much. At the very least I'd do: foreach my $path (@_) { >+ if (my $path = existOnce(@path)) { >+ ok(1, "$path exists"); >+ } else { Have you tested that? I'm pretty sure that the assignment will return true even if it's assigning the value of '0' to $path (it seems like I got bit by that before). Also, that else should be on the next line (ie, not nested). >+ ok(0, "neither ".join(' nor ',@path)." do exist --ERROR"); This error doesn't seem to be grammatically correct. Maybe something like: ok(0, "$file cannot be located --ERROR"); print $fh "Looked in:\n " . join("\n ", @path); >+ for my $I (0 .. $#{$Support::Templates::actual_files{$include_path}}) { >+ my $file = $Support::Templates::actual_files{$include_path}[$I]; >+ my $path = File::Spec->catfile($include_path, $file); >+ if (-e $path) { >+ my ($data, $err) = $provider->fetch($file); >+ >+ if (!$err) { >+ ok(1, "$file syntax ok"); >+ } >+ else { >+ ok(0, "$file has bad syntax --ERROR"); >+ print $fh $data . "\n"; >+ } I don't see $I being used anywhere but in the assignment of $file. Wouldn't: foreach my $file (@{$Support::Templates::actual_files{$include_path}}) { make more sense? Also, if you add @actual_files to what's exported from Support::Templates then you can even leave the package name out (which is the only reason to export the other variables in the first place). >+ for my $I (0 .. $#{$Support::Templates::actual_files{$include_path}}) { >+ my $file = $Support::Templates::actual_files{$include_path}[$I]; >+ my $path = File::Spec->catfile($include_path, $file); >+ open(TMPL, $path); >+ my $firstline = <TMPL>; >+ if ($firstline =~ /\d+\.\d+\@[\w\.-]+/) { >+ ok(1,"$file has a version string"); >+ } else { >+ ok(0,"$file does not have a version string --ERROR"); >+ } >+ close(TMPL); Ditto. >Index: t/Support/Templates.pm >=================================================================== >-use vars qw($include_path @referenced_files @actual_files); >+use vars qw(@languages @include_paths %include_path @referenced_files >+ %actual_files $num_actual_files); I'm pretty sure that if you export as suggested above you don't have to put them in the |use vars| pragma. (Souldn't we be using 'our' now that we require 5.6?) >+# Local subroutine used with File::Find >+sub find_include_path { >+ my ($dev,$ino,$mode,$nlink,$uid,$gid); >+ if( $File::Find::name =~ m/^[^\/]*\/([a-z-]*)$/i >+ && $_ ne 'CVS' && -d $_) { >+ push(@languages, $_) if(-d "$_/custom" or -d "$_/default"); >+ my @dir = (); >+ push(@dir, "template/$_/custom") if(-d "$_/custom"); >+ push(@dir, "template/$_/default") if(-d "$_/default"); >+ push(@include_paths, @dir); >+ $include_path{$_} = join(":",@dir); >+ } >+} >+ Shouldn't these be using File::Spec->catfile? >+my @files; > ... >- push(@actual_files, $filename); >+ push(@files, $filename); > ... >+# Scan the given template include path for templates >+sub find_actual_files { >+ my $include_path = $_[0]; >+ @files = (); >+ find(\&find_templates, $include_path); >+ return @files; >+} I realize that I'm only looking at this in patch form, but it doesn't seem to make much sense that you declare @files, you fill @files, then in the sub you empty @files and apparently never use the other stuff you collected. >+ $num_actual_files += $#{$actual_files{$include_path}} + 1; That should really be: $num_actual_file += scalar(@{$actual_files{$include_path}}) scalar(@array) returns the number of elements in the array whereas $#array returns the index of the highest element. scalar(@array) == $#array + 1 # <--- Is almost always true and in all honesty, the scalar() part is assumed if you use the @array in scalar conext, but I generally don't like assumptions :) >- open (FILE, $file); >+ open (FILE, $file); I assume this is some kind of a whitespace only change? >+ and this, too? I think that's all I have...
Updated•22 years ago
|
Blocks: bz-russian
Assignee | ||
Updated•22 years ago
|
Attachment #107354 -
Flags: review?(zach)
Assignee | ||
Comment 59•22 years ago
|
||
Finally rediffed the patch, now that bug 173622 [Move template handling into a module] is fixed. And I finally understood RFC1766/2616 (in the W3C commented version): Someone who wants English ('en') doesn't care whether (s)he get British, American, Australian,... English. But if one wants to get 'en-gb', offering 'en-us' is a no-no. - Thus this is fixed as well. t/* update comes soon.
Attachment #107354 -
Attachment is obsolete: true
Assignee | ||
Comment 60•22 years ago
|
||
> > This should be ->catdir, rather than catpath > Fixed this with all implications. Hmm, somehow it didn't survive. Now uses (finally) "catdir"! Regarding Jake's t/* directory comments: > Instead of doing all this, you can export these variables from > Support::Templates Did so now, though it makes some variables appear a bit out of nowhere this way. > >+ if (my $path = existOnce(@path)) { > >+ ok(1, "$path exists"); > >+ } else { > Have you tested that? I think I had, I tested it again and it actually seems to work. > >+ ok(0, "neither ".join(' nor ',@path)." do exist --ERROR"); Changed to suggested wording. > I don't see $I being used anywhere but in the assignment of $file. Wouldn't: > foreach my $file (@{$Support::Templates::actual_files{$include_path}}) { I now used the suggested construction. > Shouldn't these be using File::Spec->catfile? Changed this section completely and brought it line with checksetup.pl (which uses catdir (sic!))
Attachment #109942 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #109949 -
Flags: review?(bbaetz)
Assignee | ||
Updated•22 years ago
|
Attachment #109949 -
Flags: review?(jake)
Comment 61•22 years ago
|
||
(I'll re-review this once bbaetz or jake is happy with it.) Gerv
Assignee | ||
Comment 62•22 years ago
|
||
PING Anyone who wants to (re)review? (t/006spellcheck.t will fail due to be better fix from bug 166481; ignore that.)
Assignee | ||
Comment 63•22 years ago
|
||
Comment on attachment 109949 [details] [diff] [review] [real!] Eighteenth version: Fix bug + include updated t/* Gerv: > (I'll re-review this once bbaetz or jake is happy with it.) Ok, but I added you nevertheless as addl. review
Attachment #109949 -
Flags: review?(gerv)
Comment 64•22 years ago
|
||
I'd like to find the time to re-review this, but college is kicking my butt right now. In fact, I need to be starting my English paper that's due tomorrow instead of typing this comment...
Assignee: justdave → burnus
Status: REOPENED → NEW
Comment 65•22 years ago
|
||
Comment on attachment 109949 [details] [diff] [review] [real!] Eighteenth version: Fix bug + include updated t/* I'm bored of this bug. r=gerv. If no-one else checks it in, I will in the next couple of days. :-) Gerv
Attachment #109949 -
Flags: review?(gerv) → review+
Updated•22 years ago
|
Attachment #109949 -
Flags: review?(jake)
Attachment #109949 -
Flags: review?(bbaetz)
Updated•22 years ago
|
Flags: approval? → approval+
Comment 67•22 years ago
|
||
Fixed. Checking in defparams.pl; /cvsroot/mozilla/webtools/bugzilla/defparams.pl,v <-- defparams.pl new revision: 1.110; previous revision: 1.109 done Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.218; previous revision: 1.217 done Checking in Bugzilla/Template.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v <-- Template.pm new revision: 1.2; previous revision: 1.1 done Checking in t/004template.t; /cvsroot/mozilla/webtools/bugzilla/t/004template.t,v <-- 004template.t new revision: 1.24; previous revision: 1.23 done Checking in t/005no_tabs.t; /cvsroot/mozilla/webtools/bugzilla/t/005no_tabs.t,v <-- 005no_tabs.t new revision: 1.12; previous revision: 1.11 done Checking in t/Support/Templates.pm; /cvsroot/mozilla/webtools/bugzilla/t/Support/Templates.pm,v <-- Templates.pm new revision: 1.12; previous revision: 1.11 done Gerv
Status: NEW → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Comment 68•22 years ago
|
||
Oh - the change to 006spellcheck didn't apply, but it looked like the problem, whatever it was, was already fixed. Gerv
Comment 69•22 years ago
|
||
Bleh. The my $var in the template isn't modperl safe. I suppose I should have reviewed this earlier.... Don't worry about it, though; at some point someone (ie me) will need to audit all this stuff anyway.
Comment 70•22 years ago
|
||
We're going to wind up putting out a 2.16.3 anyway, because of various other issues that have come up. As long as we're doing it... most of the template sets that are starting to pop up are against the Bugzilla 2.16 branch, despite the fact that you can't actually use them in 2.16.x without patching it. We might as well make life easy on everyone and make it official. Tobias, I'm pretty sure you have a version of this patch for the 2.16 branch already that you've been distributing with the bugzilla-de templates. Any chance we can get that contributed? Does it have equivalent functionality, or is it just a "make it work?"
Whiteboard: [wanted for 2.16.3]
Comment 71•22 years ago
|
||
ok, on second thought, let's not. the size of this patch is pretty huge, and the stuff distributed with all the templates I've checked out now basically says to hard-code it into your template search path and/or to apply the patch on this bug (which won't apply to 2.16 cleanly - go figure :) we seem to be leaning towards doing a 2.18 fairly soon anyway.
Whiteboard: [wanted for 2.16.3]
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•