Closed Bug 123030 Opened 21 years ago Closed 19 years ago
Spun off from bug 96983 because of something myk also needs in bug 98801. This is a simple bug to remove the JS from the code and into a separate JS file. Ideally the JS is actually a .cgi file that generates the full JS for products for us, though this is not what 96983 asks for. I would like to know what the best approach would be: make it a .cgi file and generate the JS component arrays on the fly, or make it the responsability of the including file (possibly a template) to generate them?
Status: NEW → ASSIGNED
Without CVS write access, how do I diff for a new file (I can't do cvs add)?
I've added this as a tentative first patch. I haven't removed the actual array creation from the .atml file because I'm not sure it's the best idea. I have changed the selectProduct API to pass in four form controls (for p c v m) allowing one to pass in false as values if they don't have the corresponding control. Yes, I know the comments need to be updated. Apart from that, is it ok? Myk: I think this API suits you?
Here's the JS file, to be put in the toplevel dir. Should I use a js/ dir or something?
Myk, the problem with global/components.js.tmpl is that it doesn't actually remove inline JS from the code - just separates it perl-wise. I would have to include the file into a <script> section -- possibly the section in <head> I have included. This doesn't solve the requirements set by bug 96983 (removing <!--, cleaning up HTML). I think the proper thing to do would be to create a real .cgi that generated the values. But I'd like to hear other opinions. But isn't that bug 72837?
Attachment #67536 - Attachment description: kiko_v1_b: productmenu.ps (no cvs add here :P) → kiko_v1_b: productmenu.js (no cvs add here :P)
Depends on: 72837
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.18
This looks fine to me - has this bitrotted in the meantime, though?
*** Bug 169759 has been marked as a duplicate of this bug. ***
What needs to be done here now that bug 72837 has landed?
Kiko on IRC said move to 2.20
This is more or less a forward-port of the patches, with comments fixed and some good testing to ensure this works. In this patch I manage to reduce to a bare minimum the target milestone code -- if you don't supply the control to selectProduct, nothing happens. I moved all the JS code to where it should live: in the header. I used an anonymous block to put it where it belongs, and store the form controls in a form_controls string. It's fun! This code can be reused to do all sort of JS updating -- for instance, updating component based on product in show_bug.cgi.
Attachment #153565 - Flags: review?(myk) → review-
Comment on attachment 153830 [details] [diff] [review] kiko_v3: myk's solution is so much cleaner :-) The patch looks good, but after applying it to my local installation the target milestone field on query.cgi loses its values. Reversing the patch brings the values back. Something is going screwy there.
Attachment #153830 - Flags: review?(myk) → review-
A variable context issue, apparently.
Attachment #153830 - Attachment is obsolete: true
Attachment #153842 - Flags: review?(myk) → review+
Thanks. Great to close this one after so long! /cvsroot/mozilla/webtools/bugzilla/js/productform.js,v <-- productform.js initial revision: 1.1 /cvsroot/mozilla/webtools/bugzilla/template/en/default/search/form.html.tmpl,v <-- form.html.tmpl new revision: 1.25; previous revision: 1.24 /cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search-advanced.html.tmpl,v <-- search-advanced.html.tmpl new revision: 1.19; previous revision: 1.18
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.