560104 - rename toolkit places utils.js to PlacesUtils.jsm

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Robert Kaiser]

Citing bug 556739 comment #21:

> (In reply to bug 556739 comment #17)
> > (In reply to bug 556739 comment #15)
> > > (From update of attachment 439255 [details] [diff] [review] [details] [details])
> > > sheesh, i really should've named that file better.
> > 
> > I very much believe it would be good to rename it to PlacesUtils.jsm - but I
> > fear a lot of our code and probably add-ons use it by this suboptimal name. :(
> 
> file a bug, we could try to contact extension authors using it and notify the
> change to them before... maybe.

Comment 1

[Robert Kaiser]

We could of course ship a new minimal utils.js in 1.9.3 that loads PlacesUtils.jsm and drops a warning to the JS console telling people that "including utils.js is deprecated, use PlacesUtils.jsm instead" and then remove this utils.js on trunk after 1.9.3 branches.

Comment 2

[Dietrich Ayala (:dietrich)]

That sounds like a reasonable transition plan to me.

Comment 3

[Marco Bonardo [:mak]]

So, ship PlacesUtils.jsm, make utils.js a mock, mail extension devs about the change (or blog about it?), when amo does not show active extensions using it, remove it.
Extensions should try to cu.import PlacesUtils.jsm, if that throws then import utils.js, should work, right?

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> So, ship PlacesUtils.jsm, make utils.js a mock, mail extension devs about the
> change (or blog about it?), when amo does not show active extensions using it,
> remove it.

Right - though I'd just go and ship one release series with the mock and unconditionally remove it in the next one (actually, on trunk right after the one series with the mock has been branched).

> Extensions should try to cu.import PlacesUtils.jsm, if that throws then import
> utils.js, should work, right?

That sounds like a good way if they want to support multiple Mozilla platform versions - we should just make sure we try ourselves that it really works before we publish it.

Comment 5

[Marco Bonardo [:mak]]

Attachment: patch v1.0

This was easier than expected, i also verified that importing utils.js prints the error to the console, and that the migration code to support both versions works (it is also documented in utils.js and i'll blog about it).

Comment 6

[Marco Bonardo [:mak]]

reminder: file a bug to notify Weave about the change

Comment 7

[Dietrich Ayala (:dietrich)]

Comment on attachment 440357 [details] [diff] [review]
patch v1.0

># HG changeset patch
># User Marco Bonardo <[e-mail]>
># Date 1271801776 -7200
># Node ID aca657e81f0ad8a9f2fe140aa3d3500535dc0878
># Parent  11b0a7ea20bf8aa16d04c9e39f254e5627ffc3d7
>Bug 560104 - rename toolkit places utils.js to PlacesUtils.jsm
>
>diff --git a/browser/base/content/sanitize.xul b/browser/base/content/sanitize.xul
>--- a/browser/base/content/sanitize.xul
>+++ b/browser/base/content/sanitize.xul
>@@ -78,7 +78,7 @@
>     <script type="application/javascript"
>             src="chrome://browser/content/places/treeView.js"/>
>     <script type="application/javascript"><![CDATA[
>-      Components.utils.import("resource://gre/modules/utils.js");
>+      Components.utils.import("resource://gre/modules/PlacesUtils.jsm");
>       Components.utils.import("resource:///modules/PlacesUIUtils.jsm");
>     ]]></script>
> #endif

would there be any point in making these lazy getters?

r=me

Comment 8

[Marco Bonardo [:mak]]

well, that window is specialized on clearing visits, thus it is more likely that opening the window means using Places. So, i don't expect us having an interesting gain in doing that there.

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/6d2b17235d90

Comment 10

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-SM) Update SeaMonkey too [Checkin: Comment 13]

Comment 11

[Marco Bonardo [:mak]]

filed bug 561839 on Weave, will blog soon to notify the change to add-on developers.

Comment 12

[Robert Kaiser]

Comment on attachment 441592 [details] [diff] [review]
(Bv1-SM) Update SeaMonkey too
[Checkin: Comment 13]

I don't think this is on the right bug and it probably bitrots my own places patching, and that file will be obsoleted and killed before the 2.1 release, and I would have looked into this myself, but it's the right patch for this very moment, even though it just removes a warning.

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 441592 [details] [diff] [review]
(Bv1-SM) Update SeaMonkey too
[Checkin: Comment 13]


http://hg.mozilla.org/comm-central/rev/033053b347a4