There should be a XUL version of the duplicates report.

You can see this implementation running on b.m.o: http://bugzilla.mozilla.org/duplicates.xul Note that the page relies on the presence of a duplicates.rdf at the same directory level for the default report. On b.m.o, we generate this file with the following cron job (which runs after collectstats.pl has finished): 15 0 * * * wget -O[path to Bugzilla installation directory]/duplicates.rdf 'http://bugzilla.mozilla.org/duplicates.cgi?format=rdf'

OK, this is quite scary :-) Is this part of Bugxula? Are you about to file XUL bugs for every CGI, or did you just do this one? Gerv

>Is this part of Bugxula? Are you about to file XUL bugs for every CGI, or did >you just do this one? No, this is not part of Bugxula. No, I'm not about to file XUL bugs for every CGI; I just did this one.

Fair enough. It's just rather out of the blue, that's all. Is it because you are still concerned about the performance of duplicates.cgi? Gerv

I originally came up with the idea as a way of improving the performance of the report, both for the server and for end-users (who don't have to reload the data each time they reload the page or re-sort). The XUL version has the added benefit of being reconfigurable (columns can be resized, shown/hidden, and reordered, and column state is saved across page loads). Plus it's a good example of remote XUL and thus a good showcase for Mozilla technologies. A few notes: 1. You can specify URL parameters (f.e. product) as with the HTML version. 2. Because the code isn't signed, and needs to access some resources to which Mozilla thinks access should be restricted, you need to add the following line to your prefs.js file (shut down Mozilla first) to get this to work (this doesn't compromise your security, it merely causes Mozilla to ask you whether or not to grant access each time you load an unsigned access-seeking script): user_pref("signed.applets.codebase_principal_support", true);

1) is a bit of a disadvantage; being able to restrict it by product is very useful. Is there any way to achieve that on the client side? 2) If it's not a security hole, why don't Mozilla nightly builds ship with the pref on? :-) Gerv

>1) is a bit of a disadvantage; being able to restrict it by product is very >useful. Hunh? Why would it be a disadvantage for the XUL version to have this feature if it's very useful? >Is there any way to achieve that on the client side? Probably. >2) If it's not a security hole, why don't Mozilla nightly builds ship with the >pref on? :-) Good question. The default behavior for Mozilla builds is to silently fail to run unsigned scripts requesting access to "privileged" features. I'd expect Mozilla to warn me about running unsigned scripts rather than failing and give me the option whether or not to run the script (as it does when I set the pref), but perhaps the default behavior is targeted to the end-user masses.

Sorry, I misread your 1). It works - cool :-) Re: 2). What does mstoltz have to say? Gerv

This version fixes the bug that causes the selection to disappear when a selected bug report is loaded in the content iframe.

A few more questions :-) - I assume this is for checkin to main Bugzilla? - Should we be promoting consistency of calling by making this accessible as duplicates.cgi?ctype=xul ? i.e. making it a template? - What happened to your proposal to split format from ctype? - The Bugzilla root directory already has a load of files in. Should we take this opportunity to not bloat it further by creating "scripts" and "style" subdirectories for the .js and .css files? We could also have "images". Or are we going to do all stylesheets as templates too, so that they can also be customised in a "custom" directory without the danger of overwriting? Gerv

>- I assume this is for checkin to main Bugzilla? Yes, there's nothing b.m.o-specific about it. >- Should we be promoting consistency of calling by making this accessible as >duplicates.cgi?ctype=xul ? i.e. making it a template? In order to do that we'd need to hack duplicates.cgi to return the generated page without processing data first, since the XUL version's data is separate from its page. Even so, it's more expensive to run the CGI than return the static XUL, and doing so will make it more difficult (impossible?) to sign the scripts and may defeat useful browser caching. Overall, I think it's better for it to be a separate page, although it might be useful from a UI perspective to add a redirect from duplicates.cgi. >- What happened to your proposal to split format from ctype? That's bug 146945, one of several listed in the "format improvements" meta bug 150049. >- The Bugzilla root directory already has a load of files in. Should we take >this opportunity to not bloat it further by creating "scripts" and "style" >subdirectories for the .js and .css files? No, we should do this work separately once we have some more different kinds of files and a clearer sense of where it's all going. Any system we try to set up now is bound to be wrong. >Or are we going to do all stylesheets as templates too, so that they can also >be customised in a "custom" directory without the danger of overwriting? There's been some discussion on this but no resolution as far as I know. Time will tell, but I think we should hold out on dynamic (template-generated) stylesheets as long as possible. We can generate style references dynamically in the HTML files, and that makes things a lot simpler (because all the dynamic code is in a single file).

I don't know how much control you have over default column widths, but count, delta and TM are all too wide (and, consqequently, summary is too narrow.) Also, I think there's a case for having summary further over to the left, if possible - it's more important As more bugs load in the list, the size of that part of the window expands. Can it be made fixed? Is there any way of giving the window a title? Gerv

> Overall, I think it's better for it to be a separate page, although it might > be useful from a UI perspective to add a redirect from duplicates.cgi. That would be a good compromise. Present it as another format, but have the actual page be a redirect to the XUL URL. Does this require decoupling formats and content-types? > No, we should do this work separately once we have some more different kinds > of files and a clearer sense of where it's all going. Any system we try to > set up now is bound to be wrong. I don't agree it's "bound to be wrong"; and changing it later for .xul files breaks people's bookmarks. I can see we could move JS and CSS and images later if we liked. Does the XUL file require localisation? If so, how do we handle that? > stylesheets as long as possible. We can generate style references > dynamically in the HTML files, and that makes things a lot simpler (because > all the dynamic code is in a single file). True; but this might well lead to repetition across two sheets for the same page. I suppose you can abstract the common bits into yet another sheet. Gerv

>I don't know how much control you have over default column widths, but count, >delta and TM are all too wide (and, consqequently, summary is too narrow.) >Also, I think there's a case for having summary further over to the left, if >possible - it's more important This version narrows the ID, count and delta columns. TM is wide because its column header is big, but extra space could probably obtained by reducing CSS margins around it. Summary is in the same place it is in duplicates.cgi; if it should be someplace else we should change it for the CGI as well. Note also that these settings can be changed by individual users and will persist across those users' sessions, so we don't need (and shouldn't expect) to get it perfect. >As more bugs load in the list, the size of that part of the window expands. Can >it be made fixed? Hunh? >Is there any way of giving the window a title? Yes, by fixing bug 126903. >That would be a good compromise. Present it as another format, but have the >actual page be a redirect to the XUL URL. Does this require decoupling formats >and content-types? Nope, but it does require hacking duplicates.cgi to shortcut out of processing data and redirect immediately if the user asks for format=xul. >Does the XUL file require localisation? If so, how do we handle that? We can handle localization the same way Mozilla does; with DTD files. >True; but this might well lead to repetition across two sheets for the same >page. I suppose you can abstract the common bits into yet another sheet. I was thinking more along the lines of conditional classes like what buglist.cgi does on this line: http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/template/en/default/list/table.html.tmpl#125

> > As more bugs load in the list, the size of that part of the window expands. > > Can it be made fixed? > > Hunh? The top part of the window displays a list of bugs. This list gets populated as data arrives from the server. As it gets populated, the height of the top part of the window increases - i.e. the separator moves downwards. Any idea why, and how to stop it? > Nope, but it does require hacking duplicates.cgi to shortcut out of > processing data and redirect immediately if the user asks for format=xul. That seems reasonable. This made me think of another point: is the daysbefore value for the delta fixed in this system (because the data is in duplicates.rdf)? That would be an argument for generating the RDF data on the fly, if it's not a performance hit. > > Does the XUL file require localisation? If so, how do we handle that? > > We can handle localization the same way Mozilla does; with DTD files. Let me put it another way? Does the patch have this feature? If not, I think it should (if nothing else, to prove that remote .dtd files works.) Gerv

>The top part of the window displays a list of bugs. This list gets populated as >data arrives from the server. As it gets populated, the height of the top part >of the window increases - i.e. the separator moves downwards. Any idea why, and >how to stop it? Nope, and I can't reproduce that on my machine (nor could I when you first reported it). Maybe it was a bug fixed in Mozilla, however. Can you still reproduce? >> Nope, but it does require hacking duplicates.cgi to shortcut out of >> processing data and redirect immediately if the user asks for format=xul. > >That seems reasonable. Done. >This made me think of another point: is the daysbefore value for the delta >fixed in this system (because the data is in duplicates.rdf)? That would be an >argument for generating the RDF data on the fly, if it's not a performance hit. duplicates.xul takes the same parameters as duplicates.cgi (although it does not contain UI for them). It calls duplicates.cgi for dynamic generation if parameters exist; otherwise it uses the statically generated duplicates.rdf file for better performance. >> We can handle localization the same way Mozilla does; with DTD files. > >Let me put it another way? Does the patch have this feature? If not, I think it >should (if nothing else, to prove that remote .dtd files works.) I spoke too soon. It turns out Mozilla doesn't support remote DTD files, but I've separated out localizable text into a DTD fragment embedded at the top of the XUL document to make it easier to localize. Installations can also use content negotiation to make multiple localized copies available transparently. Other changes: 1. Removed dependency on localconfig.js, which makes it technically possible to distribute a single signed version of the script to installations other than b.m.o (the dependency make it necessary for each installation to sign their own version). 2. Added license headers to all files. 3. Updated RDF syntax to latest specifications. Ready for re-review.

I can't get this to work :-( Calling duplicates.cgi?format=rdf works, and produces the right file. Calling duplicates.cgi?ctype=xul produces the UI but no data, even after I generated an rdf file by hand and put it in the root Bugzilla directory (see below about collectstats.pl). Moz 2002-10-15 Linux. This does raise the question that people will need to use collectstats in order to use duplicates.xul. One way around this would be to get the JS to ask for the dynamic URL if it got a 404 from the static one. Is that possible? - You get two security popups when the page loads; could we get the RDF service inside loadData in order to share the privs request and only have one popup? Reading the patch by hand, it generally looks good, but I notice: - You've missed out the collectstats.pl patch to regenerate duplicates.rdf every night. - The static duplicates.rdf should be generated in the data directory, or in data/duplicates. - IMHO, duplicates.xul should be a template, in template/en/default/reports. This means we can take advantage of template l10n mechanisms automatically for the strings; we don't require content-negotiation elsewhere, and this is an output format template. I don't think this will be a significant performance hit - if templates are that expensive, we need to start patching TT :-) It's also logically consistent - duplicates.html.tmpl, duplicates.rdf.tmpl and duplicates.xul(.tmpl) are next to each other, as different views of the same data. - We should start having subdirectories off the Bugzilla directory for CSS and JS, to try and stop the untidiness in the root getting any worse. /style/ and /scripts/? - We need to add a documentation note that people will have to configure the XUL MIME type in their /etc/mime.types . Gerv

>I can't get this to work :-( Calling duplicates.cgi?format=rdf works, and >produces the right file. Calling duplicates.cgi?ctype=xul produces the UI but >no data, even after I generated an rdf file by hand and put it in the root >Bugzilla directory (see below about collectstats.pl). Moz 2002-10-15 Linux. Try it again now; I've fixed some bugs that could have caused you problems. Make sure to generate your duplicates.rdf file in the data/ directory, f.e.: wget -O/path/to/data/duplicates.rdf 'http://foo.com/duplicates.cgi?ctype=rdf' Also much sure data/.htaccess contains: <Files duplicates.rdf> allow from all </Files> Running checksetup.pl should give this to you but maybe won't if you have an existing .htaccess file. >This does raise the question that people will need to use collectstats in order >to use duplicates.xul. True, but that's true of duplicates.cgi as well, isn't it? >One way around this would be to get the JS to ask for >the dynamic URL if it got a 404 from the static one. Is that possible? That's a good idea, and this patch contains an implementation of it, but unfortunately that implementation doesn't work due to Mozilla bug 177223. When that bug is fixed we'll get this behavior automagically. >- You get two security popups when the page loads; could we get the RDF service >inside loadData in order to share the privs request and only have one popup? Good point, done. >- You've missed out the collectstats.pl patch to regenerate duplicates.rdf >every night. That's because it's not a patch on collectstats.pl but rather a cron job. >- The static duplicates.rdf should be generated in the data directory, or in >data/duplicates. Good idea, done. I put it into the data directory since it makes sense there, makes for shorter URLs, and protects us against the possibility of the duplicates directory going away. >- IMHO, duplicates.xul should be a template Although duplicates.xul relies on the CGI script for its data, I think it is otherwise closer to re-implementation of the report script than an alternate format for its data, so I think it belongs as an independent script. Regardless of which approach is better, however, we can't make it a template, because it needs to packaged as a file into a signed JAR along with its scripts in order to work for users who don't hack their prefs.js with codebase principals support (i.e. most users). The only other option is for someone to fix bug 122846, and a core RDF hacker recently told me that isn't likely to happen soon if at all. But even then we should probably be signing the script. >- We should start having subdirectories off the Bugzilla directory for CSS and >JS, to try and stop the untidiness in the root getting any worse. /style/ and >/scripts/? "style" and "logic" would be consistent but obscure, "appearance" and "behavior" would be correct but unwieldy, and "css" and "js" would be simple but limiting. Pick your poison (I like the first set best). >- We need to add a documentation note that people will have to configure the >XUL MIME type in their /etc/mime.types . Bug 177224 filed on that.

> Try it again now; I've fixed some bugs that could have caused you problems. > Make sure to generate your duplicates.rdf file in the data/ directory, f.e.: Still no luck. Having a duplicates file or not doesn't seem to make a difference - alert()s in the loadData() function all fire, and the resultsTree seems to exist, even if there's no file to read. Is there any way I can use the JS Debugger or DOM Inspector to see what might be going wrong? I've tried the DOM Inspector but it seems to stop working when you ask for privileges. > >This does raise the question that people will need to use collectstats in > > order to use duplicates.xul. > > True, but that's true of duplicates.cgi as well, isn't it? Doh! Yeah... > >- You get two security popups when the page loads; could we get the RDF > > service inside loadData in order to share the privs request and only have > > one popup? > > Good point, done. I still get two sometimes... > >- You've missed out the collectstats.pl patch to regenerate duplicates.rdf > >every night. > > That's because it's not a patch on collectstats.pl but rather a cron job. Why make admins set up another cron job? If collectstats.pl is required for all this to work, can't we also use it to generate the RDF file? >- IMHO, duplicates.xul should be a template > Regardless of which approach is better, however, we can't make it a template, > because it needs to packaged as a file into a signed JAR along with its scripts > in order to work for users who don't hack their prefs.js with codebase > principals support (i.e. most users). So you don't even get the warning popups if you haven't made that change? Can we implement our own l10n mechanism? Something like: - fetch text file from webserver over HTTP - use DOM to update labels on tree columns > "style" and "logic" would be consistent but obscure, "appearance" and > "behavior" would be correct but unwieldy, and "css" and "js" would be simple > but limiting. Pick your poison (I like the first set best). When enough browsers support other style mechanisms than CSS, or other logic mechanisms than JS, I think we can create new top level directories called XSLT and Python ;-) I'd go for css and js - simple, easy to understand. Gerv

>Still no luck. Having a duplicates file or not doesn't seem to make a >difference - alert()s in the loadData() function all fire, and the resultsTree >seems to exist, even if there's no file to read. Is there any way I can use the >JS Debugger or DOM Inspector to see what might be going wrong? I've tried the >DOM Inspector but it seems to stop working when you ask for privileges. As long as you've enabled codebase principles the number one problem is making sure the URL for the RDF file is correct and that the URL added to the tree's "ref" attribute matches the URL by which the report is identified in the "rdf:about" attribute of the "bz:duplicates_report" tag in the RDF file. Unfortunately debugging XUL templates is hideously difficult because of the number of things that can go wrong and the lack of debugging information or mechanisms. There's a bug on that somewhere. >I still get two sometimes... I need to add a call to each function that accesses restricted code, and I had to add a function like that in this iteration (even though I removed a call as well). >Why make admins set up another cron job? If collectstats.pl is required for all >this to work, can't we also use it to generate the RDF file? Aah, I didn't think of that. Ok, I'll see what I can do with my next patch. >So you don't even get the warning popups if you haven't made that change? Right, you don't get any warnings, it just doesn't work. >Can we implement our own l10n mechanism? We could, but I think it makes more sense to use existing ones (i.e. content negotiation) since they're easy to set up and use, don't require us to reinvent the wheel, and will make it easier to move to a remote DTD mechanism when one is created (since such a mechanism is likely to rely on content negotiation). >When enough browsers support other style mechanisms than CSS, or other logic >mechanisms than JS, I think we can create new top level directories called XSLT >and Python ;-) I'd go for css and js - simple, easy to understand. Sounds good to me. I've sent an email to the developers list asking for confirmation before adding these dirs.

Here's the patch with the JS file in js/ and the CSS file in css/.

[These comments relate to v5] I think your rdf template still references data/duplicates/duplicates.rdf . But, even making this fix, and updating my URLBase to reflect reality (a lot of people, me included, normally don't bother), I can't get the static stuff to work. If I append &foo=bar to the .xul URL, the dynamic stuff works fine. For static: my access log says that data/duplicates.rdf is not even being requested most of the time. The StaticDataSourceObserver onerror handler doesn't fire, presumably because there's no load attempt to error out. loadData does run to completion, and the two URLs (dataURL and the ref URL) match, and are equal to the path to duplicates.rdf. > >Can we implement our own l10n mechanism? > > We could, but I think it makes more sense to use existing ones > (i.e. content negotiation) since they're easy to set up and use, > don't require us to reinvent the wheel, and will make it easier > to move to a remote DTD mechanism when one is created (since such > a mechanism is likely to rely on content negotiation). One of the good things about our current l10n mechanism is that we've avoided having to set up content negotiation. It would be a shame to require it for this one thing. An l10n mechanism could be as simple as: - Read the contents of this URL into this array - For each item in the array, set the relevant attribute of the next column to it. Lastly, if you are moving to collectstats.pl from a cron job, there's a comment in duplicates.js which will also need to change. Gerv

>I think your rdf template still references data/duplicates/duplicates.rdf . Fixed. >For static: my access log says that data/duplicates.rdf is not even being >requested most of the time. Check your error log and make sure your data/.htaccess file exempts duplicates.rdf from the "deny from all" restrictions. >The StaticDataSourceObserver onerror handler doesn't fire, presumably because there's no load attempt to error out. Nope, it's because of Mozilla bug 177223. Oops, I thought I had mentioned that in an earlier comment; I see now that I neglected to. :-/ >One of the good things about our current l10n mechanism is that we've avoided >having to set up content negotiation. It would be a shame to require it for >this one thing. An l10n mechanism could be as simple as: >- Read the contents of this URL into this array >- For each item in the array, set the relevant attribute of the next column >to it. If our l10n story is bug 126955, then we do use content negotiation (i.e. the Accept-Language header), and remote DTDs are a better solution for XUL, so we should do what makes it easiest to migrate to them, which is the current approach. >Lastly, if you are moving to collectstats.pl from a cron job, there's a comment >in duplicates.js which will also need to change. Moved over to collectstats.pl and comments changed.

> Check your error log and make sure your data/.htaccess file > exempts duplicates.rdf from the "deny from all" restrictions. OK, the access log is now consistently showing requests for data/duplicates.rdf - the first is 200, the rest are 304 (as you might imagine.) Nothing in the error log apart from "$::buffer used only once" (you might want to fix that.) > Nope, it's because of Mozilla bug 177223. Oops, I thought I had mentioned > that in an earlier comment; I see now that I neglected to. :-/ No, you did in comment 19 - I just wasn't paying attention :-) I think it would be probably easier for me to test this if it were checked in; it obviously works for you, and mostly works for me, so that's not a problem. However, I do have a few more review comments: Your generation of duplicates.cgi is Unix-specific; it uses a / (probably easily avoided) and also uses tail. Can you just open duplicates.cgi as a pipe, discard the first three lines, then open a file and write the rest? I know we don't work on Win32 ATM - but there's no need to compound the problem :-) The other thing is that in the past (have I made this point before), we've regretted it if we have more than one "name" for a field. With that in mind, could you rename the tag names in the RDF to be the same as the database column names (op_sys, bug_severity etc.)? Gerv

>Nothing in the error log apart from "$::buffer used only once" >(you might want to fix that.) Good catch, done, and I also fixed a warning in CGI.pl due to a variable not being defined when running duplicates.cgi locally (i.e. not as a CGI). >No, you did in comment 19 - I just wasn't paying attention :-) Heh, and I even searched this bug for that number before saying it wasn't there. :-> >Your generation of duplicates.cgi is Unix-specific; it uses a / (probably >easily avoided) and also uses tail. Can you just open duplicates.cgi >as a pipe, discard the first three lines, then open a file and write the rest? Yes, done, but I continue to use "/" since those work in Windows Perl. >The other thing is that in the past (have I made this point before), we've >regretted it if we have more than one "name" for a field. With that in mind, >could you rename the tag names in the RDF to be the same as the database >column names (op_sys, bug_severity etc.)? I use the same names the RDF output to buglist.cgi uses, and they're more sensible (especially "severity" for "bugs.bug_severity"), so a better solution for the inconsistency is to normalize the database names (for which I filed bug 177692).

Comment on attachment 104750 [details] [diff] [review] patch v8: review fixes r=gerv - looks pretty good. Just because it doesn't work for me yet doesn't mean it shouldn't go in. Let's test it out on b.m.o., and then we'll see if it really works :-) Gerv

Checking in duplicates.cgi; /cvsroot/mozilla/webtools/bugzilla/duplicates.cgi,v <-- duplicates.cgi new revision: 1.29; previous revision: 1.28 done Checking in collectstats.pl; /cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v <-- collectstats.pl new revision: 1.23; previous revision: 1.22 done Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.202; previous revision: 1.201 done Checking in CGI.pl; /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v <-- CGI.pl new revision: 1.189; previous revision: 1.188 done RCS file: /cvsroot/mozilla/webtools/bugzilla/duplicates.xul,v done Checking in duplicates.xul; /cvsroot/mozilla/webtools/bugzilla/duplicates.xul,v <-- duplicates.xul initial revision: 1.1 done RCS file: /cvsroot/mozilla/webtools/bugzilla/js/duplicates.js,v done Checking in js/duplicates.js; /cvsroot/mozilla/webtools/bugzilla/js/duplicates.js,v <-- duplicates.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/webtools/bugzilla/css/duplicates.css,v done Checking in css/duplicates.css; /cvsroot/mozilla/webtools/bugzilla/css/duplicates.css,v <-- duplicates.css initial revision: 1.1 done RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates.rdf.tmpl,v done Checking in template/en/default/reports/duplicates.rdf.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates.rdf.tmpl,v <-- duplicates.rdf.tmpl initial revision: 1.1 done

fixing target