Closed
Bug 160557
(bugzillaunderscore)
Opened 22 years ago
Closed 22 years ago
products that start with _ do not show up properly in query.cgi
Categories
(Bugzilla :: Query/Bug List, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: pbaker, Assigned: pbaker)
References
Details
Attachments
(1 file, 2 obsolete files)
4.02 KB,
patch
|
bbaetz
:
review+
bbaetz
:
review+
|
Details | Diff | Splinter Review |
If you have a product name that starts with a underscore (_), the product's components/versions/milestones do not show up on the query.cgi page when that product is selected from the listbox. 1. create a product with a name like "_misc". 2. add some components to this product. 3. now go to the query page. 4. select the _misc (or whatever you named) it product in the list. Actual Results: the component, version, and target list boxes become empty. Expected Results: the component, version, and target list boxes should be populated with the components, version, and milestones that correspond to the _misc product. Additional Information: Looking at the html source, shows in the javascript: cpts[0] = [ ]; vers[0] = [ ]; tms[0] = [ ]; Which is why the boxes are empty. Also I put this as major because it is causing major problems for my company's installation of bugzilla because we have the product called _customer_issue which tracks all customer reported problems. We have a component for everyone of our clients (yes there are a ton of components). It is our largest product, so trying to search for customer issue bugs without limiting it to the particular customer we want is a major pain. We could change the name to just 'customer-issue' I suppose, but we have the _ in front so that it always sorts to the top of all lists. (it's used the most so it should be at the top shouldn't it.) That also doesn't change the fact that this is a bug.
Updated•22 years ago
|
Alias: bugzillaunderscore
Comment 1•22 years ago
|
||
This is a "feature" of Template Toolkit as noted in the man page for Template::Manual::Variables under the "Hash Array References" section: Any key in a hash which starts with a '_' or '.' character will be considered private and cannot be evaluated or updated from within a template. The undefined value will be returned for any such variable accessed which the Tem- plate Toolkit will silently ignore (unless the DEBUG option is enabled). The question is whether we want to allow users to create products (and components and ???) with leading underscore characters, and if so, how do we make Template Toolkit behave like we want it to.
Assignee | ||
Comment 2•22 years ago
|
||
I know how to fix this. I'm about to go into a meeting, but I'll have a patch ready soon after I get out. Should I reassign this bug to myself then I guess?
Assignee | ||
Comment 3•22 years ago
|
||
This fixes it on my bugzilla installation.
Assignee | ||
Comment 4•22 years ago
|
||
ddk says to assign it to myself since I gave the patch. so here goes...
Assignee: endico → pbaker
Comment 5•22 years ago
|
||
Reassigning to patch author.
Comment 6•22 years ago
|
||
I don't think that this patch will work. We really don't want to restrict ourselves to not using hashes, which is where this will end up (since anything can start with a _). Maybe we can find a way to disable this feature? TT uses it internally though, so probably not. Thoughts?
Assignee | ||
Comment 7•22 years ago
|
||
My original thought from many many months ago was to not use TT for the template engine, but I guess it's a little late for that...</troll> Unless you can convince TT upstream to some how make that a disableable feature, I can't think of any other way to handle this situation.
Comment 8•22 years ago
|
||
:) TT also does teh same thing for names starting with '.', probably because hash..elem stuffs up the parser. How do other tempalte systems deal with that?
Assignee | ||
Comment 9•22 years ago
|
||
Without getting into to much of a template engine flamewar, the template engine I suggested while back was HTML::Template, mainly because it does not all you to do any kind of coding in the templates like TT does. This would have meant that more dirty work would have to be done in the actual bugzilla code to use H:T instead of being able to take shortcuts in the templates. It is also more array based than hash based, so that would be one reason it wouldn't have a _|. key problem^H^H^H^H^H^H^Hfeature like TT does (see comment#2). But it's a little late to change template engines so I won't spend any more time on that. So really don't have much of an answer. Who has the best relationship talking to the TT maintainers?
Assignee | ||
Comment 10•22 years ago
|
||
In TT can we do something like this: my @products = (); foreach my $p (@legal_products) { my %p = { product => $p, components => \@comps, versions => \@vers, mstones => \@mstones, }; push @products, \%p; } $::vars{'products} = \@products; Then in the template do something like this: [% FOREACH p products %] cpts = [ [% FOREACH c p.components ]% yadda yadda... [% END %] ] [% END %] Pardon for the bad template syntax above I don't have the syntax memorized yet. But do you think that might be a doable solution? If you want I could go ahead and maybe do a new patch that does like that. Instead of 3 or 4 arrays, it would be one array of hashes.
Assignee | ||
Comment 11•22 years ago
|
||
This is an alternative patch. Instead of creating 4 arrays (products, components, versions, milestones) it creates a products array that contains a hash with values for each thing instead. It's also different from the original code that created instead 4 hashes. Not to mention it also decreases the number of lines (even with the 2 lines of comments I added) and the template looks a bit cleaner as well. Didn't mark this as obsoleting my original patch, because the both work. It's just a question of which one you guys choose...or if.
Comment 12•22 years ago
|
||
> ... I can't think of any other way to handle this situation.
Preventing Bugzilla products starting with _ comes to mind.
Comment 13•22 years ago
|
||
Comment on attachment 93649 [details] [diff] [review] alternate patch that uses array of hashes I like this interface change; the names like "componentsbyproduct" were signs we were doing something wrong anyway. The fact that it fixes the other issue is a bonus. >+for (my $i = 0; $i < @products; ++$i) { >+ my $p = $products[$i]; >+ >+ ## create hash to hold attributes for each product. >+ my %p = ( >+ name => $p, >+ components => [ sort { lc($a) cmp lc($b) } @{$::components{$p}} ], >+ versions => [ sort { lc($a) cmp lc($b) } @{$::versions{$p}} ] >+ ); >+ $p{milestones} = [ sort { lc($a) cmp lc($b) } @{$::target_milestone{$p}} ] >+ if Param('usetargetmilestone'); >+ ## assign hash back to product array. >+ $products[$i] = \%p; > } Please use Bugzilla coding style, as outlined in the Developers' Guide on the website. Specifically, single-hashes precede comments, and you should turn the "usetargetmilestone" check into a proper if clause, as it doesn't all fit on one line. Also, while you are there: - can we use %product instead of %p? - we don't need the temporary $p at all - please quote hash keys using single quotes ( $p{'milestones'} ). But this patch is generally good :-) Gerv
Attachment #93649 -
Flags: review-
Comment 14•22 years ago
|
||
Here's a version with the review nits picked. I'm not stealing your patch - it's just I want to get this in quickly in order to make further modifications to that area of code. :-) Gerv
Attachment #93615 -
Attachment is obsolete: true
Attachment #93649 -
Attachment is obsolete: true
Comment 15•22 years ago
|
||
Comment on attachment 94772 [details] [diff] [review] Patch v.1 OK, this looks good, and its cleaner, too r=bbaetz x2 I'd ask you to update the INTERFACE comments, but this template doesn't actually have one...
Attachment #94772 -
Flags: review+
Comment 16•22 years ago
|
||
Fixed. Checking in query.cgi; /cvsroot/mozilla/webtools/bugzilla/query.cgi,v <-- query.cgi new revision: 1.102; previous revision: 1.101 done Checking in template/en/default/search/form.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/search/form.html.tmpl,v <-- form.html.tmpl new revision: 1.11; previous revision: 1.10 done Gerv
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•22 years ago
|
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.18
Comment 17•21 years ago
|
||
*** Bug 196896 has been marked as a duplicate of this bug. ***
Updated•11 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
•