Remove nsIPrefBranch2 uses from comm-central

RESOLVED FIXED in Thunderbird 13.0

Status

MailNews Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

unspecified
Thunderbird 13.0
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 597568 [details] [diff] [review]
part A, calendar/
Attachment #597568 - Flags: review?(mbanner)
(Assignee)

Comment 2

6 years ago
Created attachment 597570 [details] [diff] [review]
part B, suite/
Attachment #597570 - Flags: review?(mbanner)
(Assignee)

Comment 3

6 years ago
Created attachment 597571 [details] [diff] [review]
part C, mailnews/
Attachment #597571 - Flags: review?(mbanner)
(Assignee)

Comment 4

6 years ago
Created attachment 597573 [details] [diff] [review]
part D, mail/ (ignore the editor/ bits)
Attachment #597573 - Flags: review?(mbanner)
Comment on attachment 597570 [details] [diff] [review]
part B, suite/

Review of attachment 597570 [details] [diff] [review]:
-----------------------------------------------------------------

I did not check against mxr to see if there were more uses in suite/ but this part looks good.
Attachment #597570 - Flags: review?(mbanner) → review+

Comment 6

6 years ago
Comment on attachment 597573 [details] [diff] [review]
part D, mail/ (ignore the editor/ bits)

(I only looked at editor code.)

>-      var pbi = pref.QueryInterface(Components.interfaces.nsIPrefBranch2);
>+      var pbi = pref.QueryInterface(Components.interfaces.nsIPrefBranch);
>       pbi.addObserver(this.domain, this, false);

>-      var pbi = pref.QueryInterface(Components.interfaces.nsIPrefBranch2);
>+      var pbi = pref.QueryInterface(Components.interfaces.nsIPrefBranch);
>       pbi.removeObserver(this.domain, this);
pref.add/removeObserver() suffices, no?
(Or indeed Services.prefs should work.)
(Assignee)

Comment 7

6 years ago
Created attachment 597742 [details] [diff] [review]
part E, editor/

I probably should've split this off in the first place.
Attachment #597742 - Flags: review?(neil)
(Assignee)

Updated

6 years ago
Attachment #597573 - Attachment description: part D, everything else → part D, mail/ (ignore the editor/ bits)

Updated

6 years ago
Attachment #597742 - Flags: review?(neil) → review+
(In reply to neil@parkwaycc.co.uk from comment #6)
> (Or indeed Services.prefs should work.)

As usual, I'm going to warn about blindly replacing pref with Services.prefs: <http://monogatari.doukut.su/2011/02/note-about-services-and-mailservices.html>. Especially for editor/ which isn't really tested I believe.

Comment 9

6 years ago
You can leave converting to Services and Mailservices to bug 720358 and bug 720356. I do it per directory/component and so it can hopefully get better testing.
Attachment #597568 - Flags: review?(mbanner) → review?(philipp)
Comment on attachment 597571 [details] [diff] [review]
part C, mailnews/

Review of attachment 597571 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing the patch, just a couple of nits for indentation.

::: mailnews/addrbook/src/nsAbAddressCollector.cpp
@@ +352,1 @@
>                                                      &rv));

nit: Can you fix the second line as well please, so that the &rv keeps aligning? (several places)
Attachment #597571 - Flags: review?(mbanner) → review+
Attachment #597573 - Flags: review?(mbanner) → review+
Comment on attachment 597568 [details] [diff] [review]
part A, calendar/

r=philipp for calendar
Attachment #597568 - Flags: review?(philipp) → review+
(Assignee)

Comment 12

6 years ago
Landed with those nits addressed.

http://hg.mozilla.org/comm-central/rev/f148c7fda60f
http://hg.mozilla.org/comm-central/rev/1568ea5a5309
http://hg.mozilla.org/comm-central/rev/b55594e9215d
http://hg.mozilla.org/comm-central/rev/8b3d55adf7a6
http://hg.mozilla.org/comm-central/rev/38d22eafca67
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
You need to log in before you can comment on or make changes to this bug.