916235 - Remove obsolete (and now unused) strres.js from the tree

Status: RESOLVED DUPLICATE of bug 1312143

(Toolkit Graveyard :: Build Config, defect)

Description

[Serge Gautherie (:sgautherie)]

1) Remove the following line, to stop packaging strres.js.

/mozilla/toolkit/obsolete/jar.mn
    * line 12 -- + content/global/strres.js (content/strres.js)

2) Remove actual strres.js file in Hg.

http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/obsolete/content/strres.js

Comment 1

[:Cykesiopka]

Attachment: bug916235_v1.patch

Not really sure who should review this, so please redirect as necessary...

Comment 2

[Dave Townsend [:mossop]]

I see quite a surprising number of add-ons making use of this script including some recent ones. There isn't a lot to be gained by removing this except for general cleanup wins so that doesn't seem worth it if we're going to break add-ons needlessly. Jorge what do you think? Would you rather we kept this around for now?

Comment 3

[Serge Gautherie (:sgautherie)]

Fwiw, I would expect files that live in /toolkit/obsolete/ to have been deprecated (a long time ago), so removing them now should be a matter of announcing it officially.
If that's not the case, maybe it's time to start planning for that.

Comment 4

[Jorge Villalobos [:jorgev] (he/him)]

Wow, that's a lot of add-ons using this, and just for a little shortcut...

(In reply to Serge Gautherie (:sgautherie) from comment #3)
> Fwiw, I would expect files that live in /toolkit/obsolete/ to have been
> deprecated (a long time ago), so removing them now should be a matter of
> announcing it officially.

Well, all add-on uses I see just load chrome://global/content/strres.js, so they don't know that the file is being loaded from an "obsolete" path.

I think that given the wide usage of this script, we should begin by adding a warning in the console for whenever this file is loaded. We should leave that in for a couple of releases and then finish this. If this is too much work for this file, we might as well leave it be and save us the compatibility problem.

Comment 5

[Dave Townsend [:mossop]]

(In reply to Jorge Villalobos [:jorgev] from comment #4)
> (In reply to Serge Gautherie (:sgautherie) from comment #3)
> > Fwiw, I would expect files that live in /toolkit/obsolete/ to have been
> > deprecated (a long time ago), so removing them now should be a matter of
> > announcing it officially.
> 
> Well, all add-on uses I see just load chrome://global/content/strres.js, so
> they don't know that the file is being loaded from an "obsolete" path.
> 
> I think that given the wide usage of this script, we should begin by adding
> a warning in the console for whenever this file is loaded. We should leave
> that in for a couple of releases and then finish this. If this is too much
> work for this file, we might as well leave it be and save us the
> compatibility problem.

We should probably do this for all the files in the obsolete path, some of which are unfortunately still in use in the tree. I filed bug 917024 so before we could take this patch we would have to resolve that and let it sit in the tree for a few releases.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to Dave Townsend (:Mossop) from comment #5)
> I filed bug 917024 so
> before we could take this patch we would have to resolve that and let it sit
> in the tree for a few releases.

Adding a warning now (there) then removing the file (here) a bit later is fine wrt this bug :-)

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 804992 [details] [diff] [review]
bug916235_v1.patch

The code changes here are good but we can't land them till the other work is done.

Comment 8

[:Cykesiopka]

Unassigning myself because I don't think I'll have the time to push the prerequisite Bug 917024 to completion, since my focus is currently in other areas of code.