Closed Bug 848519 Opened 7 years ago Closed 7 years ago
Move services' preferences
.js into Toolkit as Preferences .jsm
I was using preferences.js today (https://hg.mozilla.org/mozilla-central/file/default/services/common/preferences.js) and figured it might make a good addition to Toolkit. If Toolkit is interested, I'll code up a patch to land it there. We may do this in two phases: 1) Add to Toolkit with minor changes 2) Port existing consumers to use the Toolkit version. I haven't looked at the source in a while - maybe the API is already good enough to land!
(In reply to Gregory Szorc [:gps] from comment #0) > If Toolkit is interested, I'll code up a patch to land it there. Yes, please. > I haven't looked at the source in a while - maybe the API is already good enough to > land! It's already being used all over the place (including e.g. DOM code: http://hg.mozilla.org/integration/mozilla-inbound/rev/2aaf82b852e7#l14.23), so it's absolutely "good enough to land" as-is.
Nick might be tackling this as part of FHR on Android.
I think it makes sense to move services/common/preferences.js into toolkit/modules/Preferences.jsm The capital-P and jsm suffix is consistent with toolkit/modules. Sound good?
Sounds good to me!
As part of Android FHR, we are likely to ship services-common on Android. That removes my immediate desire to do this. I wonder if we should ship services' log4moz.js as well? I see only one use outside of services/, namely toolkit/modules/Sqlite.jsm.
(In reply to Nick Alexander :nalexander from comment #5) > As part of Android FHR, we are likely to ship services-common on Android. > That removes my immediate desire to do this. Why not just fix this bug instead? Solves the problem for everyone, rather than extending it. Log4moz is bug 451283, and I think that one should be trivially fixed as well.
(In reply to :Gavin Sharp (use email@example.com for email) from comment #6) > Log4moz is bug 451283, and I think that one should be trivially fixed as > well. You beat me to it. Also, I think between Gavin and myself, we can save Mossop another bug comment…
Try push running at https://tbpl.mozilla.org/?tree=Try&rev=325391616620
https://tbpl.mozilla.org/?tree=Try&rev=5b3ca2886c83 This time, with proper gre/modules!
Gah, missed one: http://tbpl.mozilla.org/?tree=Try&rev=43d3c4cec944
http://tbpl.mozilla.org/?tree=Try&rev=66574239569d Final try push to all architectures (previous one was just to Fedora to make sure I unbusted M bc correctly).
I removed preferences.js from removed-files.in, but I didn't add Preferences.jsm to any such list because I saw no other toolkit/modules files in such a list.
Assignee: nobody → nalexander
Attachment #736585 - Flags: review?(gavin.sharp)
A tab slipped into toolkit/modules/Makefile.in. This version removes it.
Comment on attachment 736586 [details] [diff] [review] Patch against m-c, v2 >diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in >- modules/services-common/preferences.js removed-files is a list of files that used to be present in the appdir but were since removed - you should essentially never remove entries from it, so don't make this change. (Eventually we can clean this up once we don't care about upgrades from versions where these files existed.) >diff --git a/services/common/tests/unit/test_preferences.js b/toolkit/modules/tests/xpcshell/test_Preferences.js Do you know why some tests are commented out? It would be good to reference bugs on file to re-enabled them.
Attachment #736586 - Flags: review?(gavin.sharp) → review+
> Do you know why some tests are commented out? It would be good to reference > bugs on file to re-enabled them. I have no idea, and gps doesn't recall. It's part of a general refactor: https://bugzilla.mozilla.org/show_bug.cgi?id=731494 I'm going to commit without touching this.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff628a483c47 https://hg.mozilla.org/integration/mozilla-inbound/rev/05cf87d0435b https://hg.mozilla.org/integration/mozilla-inbound/rev/3b681cdd0bc5
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla23
https://hg.mozilla.org/mozilla-central/rev/ff628a483c47 https://hg.mozilla.org/mozilla-central/rev/05cf87d0435b https://hg.mozilla.org/mozilla-central/rev/3b681cdd0bc5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Summary: Move services' preferences.js into Toolkit → Move services' preferences.js into Toolkit as Preferences.jsm
It would actually be nice to get this module documented on MDN. Eric, do you think that this can be done somehow in the next time? Looks like the request for it is waiting for more than 2 years now. Thanks.
You need to log in before you can comment on or make changes to this bug.