380756 - Remove duplicates.xul (was: what is CanonicaliseParams()??)

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, defect)

Description

[Frédéric Buclin]

As far as I can see, CanonicaliseParams() never existed, despite it has been introduced in bug 156548. I checked in 2.20.4 and in 3.1, and I cannot find any function with such a name:

Undefined subroutine &main::CanonicaliseParams called at /var/www/html/bugzilla/duplicates.cgi line 45.

To throw this error, all you have to do is to go to duplicates.cgi?ctype=xul. This makes me think this feature never worked.

Comment 1

[Frédéric Buclin]

Did you mean $cgi->canonicalise_query()? See Bugzilla::CGI::canonicalise_query().

Comment 2

[Frédéric Buclin]

I see, bug 147833 removed CanonicaliseParams() from CGI.pl and replaced it with Bugzilla::CGI::canonicalise_query() on Oct 25, 2002. And duplicates.cgi started using CanonicaliseParams() on Nov 4, 2002; one week later! So bbaetz couldn't fix it as it wasn't checked in yet. And probably the reviewer of duplicates.cgi didn't upgrade yet and so still had CanonicaliseParams() in his installation.

Said otherwise: this code *never* worked (even when it was implemented in 2.17.x) and nobody ever noticed it!! Probably a very useful feature. :-/

Does it mean we should kill it completely?

Comment 3

[Gervase Markham [:gerv]]

Bugzilla/CGI.pm was checked in (including canonicalise_query(), which used to be CanonicaliseParams() in the now-defunct CGI.pl) on 2002-10-25, half way through the review process for bug 156548. I strongly suspect Myk did a CVS update and then checkin without realising that a function he called had disappeared from under his feet. And it took so long for it to arrive on b.m.o. that everyone had forgotten it existed. 

Ah, well.

Gerv

Comment 4

[Max Kanat-Alexander]

I for one don't object to removing features that are never used.

If somebody wants to make an XUL interface for Bugzilla, that should be a separate project. (And was--Bugxula, a while ago, but it died.)

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Actually this feature has always worked at its canonical location, it just hasn't worked from the redirect built into duplicates.cgi.  Here's where to go to access the feature on b.m.o:

jar:http://bugzilla.mozilla.org/duplicates.jar!/duplicates.xul

Note: it can take some time for the tree to populate.

Nevertheless, I doubt the feature is used very much on b.m.o or at all on other installations, since you have to sign it (or compromise the security of your web browser) to get it working, so I'm fine with removing it.

Comment 6

[Teemu Mannermaa (:wicked)]

As the XUL version still works I don't think we should remove it. We should instead just remove or fix the broken redirection code.

Comment 7

[Gervase Markham [:gerv]]

Are we sure the XUL version still works? XUL has changed a fair bit since 2002.

Gerv

Comment 8

[Myk Melez [:myk] [@mykmelez]]

I wrote:

> Here's where to go to access the feature on b.m.o:
> 
> jar:http://bugzilla.mozilla.org/duplicates.jar!/duplicates.xul

Err, actually it's:

jar:https://bugzilla.mozilla.org/duplicates.jar!/duplicates.xul


Teemu wrote:

> As the XUL version still works I don't think we should remove it. We should
> instead just remove or fix the broken redirection code.

I think it actually is worth removing it if it isn't being used, even though it still works, given that it's unowned and that we haven't made any other moves towards building XUL-based interfaces into Bugzilla.

And I think Max is right that a XUL-based interface should be a separate project, with the core Bugzilla project providing just the standard HTML interface plus an API that other projects can use to provide alternate ones.


Gerv wrote:

> Are we sure the XUL version still works? XUL has changed a fair bit since
> 2002.

I don't think it's changed a whole lot, actually.  In any case, the parts that the Duplicates Report needs haven't changed, so it still works.

Comment 9

[Gervase Markham [:gerv]]

I just tried it; I had to trust "America On Line" twice, and then every time I load a bug, but it worked.

Gerv

Comment 10

[Myk Melez [:myk] [@mykmelez]]

> I had to trust "America On Line" twice, and then every time I load a bug

That's because AOL bought the certificate that we used to sign the code.  Alternately, you could enable codebase principals, use the unsigned version, and trust bugzilla.mozilla.org instead.

Comment 11

[Frédéric Buclin]

Attachment: patch, v1

Comment 12

[Frédéric Buclin]

Comment on attachment 321158 [details] [diff] [review]
patch, v1

myk said he doesn't have time to review this patch.

Comment 13

[Max Kanat-Alexander]

Comment on attachment 321158 [details] [diff] [review]
patch, v1

I think we should also modify checksetup to remove duplicates*.rdf.

Otherwise I'm fine with removing all this.

Comment 14

[Frédéric Buclin]

Attachment: patch, v2

Remove data/duplicates*.rdf

Comment 15

[Max Kanat-Alexander]

Comment on attachment 321530 [details] [diff] [review]
patch, v2

Looks good. Of course, old .htaccess files will continue to have the allow statement in them for duplicates.rdf.

Comment 16

[Frédéric Buclin]

(In reply to comment #15)
> (From update of attachment 321530 [details] [diff] [review])
> Looks good. Of course, old .htaccess files will continue to have the allow
> statement in them for duplicates.rdf.

Yes, but that's not a big deal as they no longer exist anyway. And this is probably safer than trying to fix .htaccess ourselves, especially if an admin edited it manually.

Comment 17

[Frédéric Buclin]

Checking in collectstats.pl;
/cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v  <--  collectstats.pl
new revision: 1.65; previous revision: 1.64
done
Checking in duplicates.cgi;
/cvsroot/mozilla/webtools/bugzilla/duplicates.cgi,v  <--  duplicates.cgi
new revision: 1.62; previous revision: 1.61
done
Removing duplicates.xul;
/cvsroot/mozilla/webtools/bugzilla/duplicates.xul,v  <--  duplicates.xul
new revision: delete; previous revision: 1.2
done
Checking in Bugzilla/Install/Filesystem.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Filesystem.pm,v  <--  Filesystem.pm
new revision: 1.30; previous revision: 1.29
done
Checking in docs/en/xml/security.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/en/xml/security.xml,v  <--  security.xml
new revision: 1.19; previous revision: 1.18
done
Removing js/duplicates.js;
/cvsroot/mozilla/webtools/bugzilla/js/duplicates.js,v  <--  duplicates.js
new revision: delete; previous revision: 1.2
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.112; previous revision: 1.111
done
Removing template/en/default/reports/duplicates.rdf.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates.rdf.tmpl,v  <--  duplicates.rdf.tmpl
new revision: delete; previous revision: 1.4
done

Comment 18

[David D. Kilzer (ddk)]

Part of the deleted code in this bug began shipping again in the 3.1.x and 3.2.x series, but was removed (yet again) in 3.3.x and later.  See Bug 501040 for more details.

Comment 19

[Frédéric Buclin]

(In reply to comment #18)
> Part of the deleted code in this bug began shipping again in the 3.1.x and
> 3.2.x series, but was removed (yet again) in 3.3.x and later.  See Bug 501040
> for more details.

This statement is completely wrong. You misunderstand what the version field means, as I explained in bug 501040.

Comment 20

[David D. Kilzer (ddk)]

(In reply to comment #19)
> This statement is completely wrong. You misunderstand what the version field
> means, as I explained in bug 501040.

Yep, my bad.

Comment 21

[Max Kanat-Alexander]

Added to the release notes for Bugzilla 3.4 in bug 494037.