Geoff Lankow (:darktrojan) Assignee
	Description • 12 years ago

It's time to get rid of nsIPrefBranch2. I'll merge it with nsIPrefBranch and leave an empty interface for compatibility.

Benjamin, are you willing to do reviews for this?

	Benjamin Smedberg
	Comment 8 • 12 years ago

I will get to this review on Thurdsay, I'm traveling this week.

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 9 • 12 years ago

Just to be sure: all these patches build on their own?

	Geoff Lankow (:darktrojan) Assignee
	Comment 10 • 12 years ago

Comment on attachment 590145 [details] [diff] [review]
part A, merge interfaces

(In reply to Ms2ger from comment #9)
> Just to be sure: all these patches build on their own?

I sent it all to TryServer every time I started a new part. Only A and B actually change how things work.

	Geoff Lankow (:darktrojan) Assignee
	Comment 11 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/e933d31eb696
https://hg.mozilla.org/integration/mozilla-inbound/rev/910cc1ad2803
https://hg.mozilla.org/integration/mozilla-inbound/rev/860513cc7d5a
https://hg.mozilla.org/integration/mozilla-inbound/rev/102dd9765964
https://hg.mozilla.org/integration/mozilla-inbound/rev/07800b5b4d83
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc98d884f0f0
https://hg.mozilla.org/integration/mozilla-inbound/rev/04ac18d14436

We should mark nsIPrefBranch2 as deprecated on MDN and in the addon validator.

	Geoff Lankow (:darktrojan) Assignee
	Comment 12 • 12 years ago

I'll need to follow this up with some new additions. Will do so after a few days in case there's stuff from other branches.

	Marco Bonardo [:mak] (Away Apr 25 - May 5)
	Comment 13 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/e933d31eb696
https://hg.mozilla.org/mozilla-central/rev/910cc1ad2803
https://hg.mozilla.org/mozilla-central/rev/860513cc7d5a
https://hg.mozilla.org/mozilla-central/rev/102dd9765964
https://hg.mozilla.org/mozilla-central/rev/07800b5b4d83
https://hg.mozilla.org/mozilla-central/rev/fc98d884f0f0
https://hg.mozilla.org/mozilla-central/rev/04ac18d14436

	Kris Maglione [:kmag]
	Comment 14 • 12 years ago

It would be nice if a compatibility warning were emitted the first time an attempt is made to QI a preference branch to nsIPrefBranch2 so code that relies on it doesn't suddenly break without warning when it goes away.

	Benjamin Smedberg
	Comment 15 • 12 years ago

kmag, I don't think we have plans to remove nsIPrefBranch2 any time soon, so I don't think that the in-product compatibility warning is needed currently.

	Geoff Lankow (:darktrojan) Assignee
	Comment 16 • 12 years ago

Last one (hopefully)

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 17 • 12 years ago

Comment on attachment 600535 [details] [diff] [review]
part H

>diff --git a/toolkit/components/places/nsPlacesAutoComplete.js b/toolkit/components/places/nsPlacesAutoComplete.js

>-    if (aRegisterObserver) {
>-      let pb = this._prefs.QueryInterface(Ci.nsIPrefBranch2);
>-      pb.addObserver("", this, false);
>-    }
>+    this._prefs.addObserver("", this, false);

Don't you need to preserve the aRegisterObserver check?

	Geoff Lankow (:darktrojan) Assignee
	Comment 18 • 12 years ago

Oh, uhh, whoops.

	Geoff Lankow (:darktrojan) Assignee
	Comment 19 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8a1e874238a

	Phil Ringnalda (:philor)
	Comment 20 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/a8a1e874238a

	Geoff Lankow (:darktrojan) Assignee
	Comment 21 • 12 years ago

I've updated these MDN pages. There's still a lot of mentions, but it would be too messy to change them at this point.

https://developer.mozilla.org/en/Code_snippets/Preferences
https://developer.mozilla.org/en/Firefox_13_for_developers
https://developer.mozilla.org/en/Preferences_API
https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIPrefBranch
https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIPrefBranch2