Last Comment Bug 727408 - Remove nsIPrefBranch2 uses from comm-central
: Remove nsIPrefBranch2 uses from comm-central
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 13.0
Assigned To: Geoff Lankow (:darktrojan)
:
Mentors:
Depends on: 718255
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-15 03:33 PST by Geoff Lankow (:darktrojan)
Modified: 2012-02-21 14:58 PST (History)
4 users (show)
geoff: in‑testsuite-
geoff: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
part A, calendar/ (19.55 KB, patch)
2012-02-15 14:49 PST, Geoff Lankow (:darktrojan)
philipp: review+
Details | Diff | Splinter Review
part B, suite/ (20.47 KB, patch)
2012-02-15 14:50 PST, Geoff Lankow (:darktrojan)
bugspam.Callek: review+
Details | Diff | Splinter Review
part C, mailnews/ (17.47 KB, patch)
2012-02-15 14:50 PST, Geoff Lankow (:darktrojan)
standard8: review+
Details | Diff | Splinter Review
part D, mail/ (ignore the editor/ bits) (20.19 KB, patch)
2012-02-15 14:51 PST, Geoff Lankow (:darktrojan)
standard8: review+
Details | Diff | Splinter Review
part E, editor/ (2.27 KB, patch)
2012-02-16 03:04 PST, Geoff Lankow (:darktrojan)
neil: review+
Details | Diff | Splinter Review

Description Geoff Lankow (:darktrojan) 2012-02-15 03:33:17 PST

    
Comment 1 Geoff Lankow (:darktrojan) 2012-02-15 14:49:41 PST
Created attachment 597568 [details] [diff] [review]
part A, calendar/
Comment 2 Geoff Lankow (:darktrojan) 2012-02-15 14:50:16 PST
Created attachment 597570 [details] [diff] [review]
part B, suite/
Comment 3 Geoff Lankow (:darktrojan) 2012-02-15 14:50:47 PST
Created attachment 597571 [details] [diff] [review]
part C, mailnews/
Comment 4 Geoff Lankow (:darktrojan) 2012-02-15 14:51:23 PST
Created attachment 597573 [details] [diff] [review]
part D, mail/ (ignore the editor/ bits)
Comment 5 Justin Wood (:Callek) 2012-02-15 15:22:54 PST
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.
Comment 6 neil@parkwaycc.co.uk 2012-02-16 02:50:25 PST
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.)
Comment 7 Geoff Lankow (:darktrojan) 2012-02-16 03:04:52 PST
Created attachment 597742 [details] [diff] [review]
part E, editor/

I probably should've split this off in the first place.
Comment 8 Siddharth Agarwal [:sid0] (inactive) 2012-02-16 04:31:27 PST
(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 :aceman 2012-02-16 07:41:00 PST
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.
Comment 10 Mark Banner (:standard8) (afk until 26th July) 2012-02-21 06:09:59 PST
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)
Comment 11 Philipp Kewisch [:Fallen] 2012-02-21 06:23:23 PST
Comment on attachment 597568 [details] [diff] [review]
part A, calendar/

r=philipp for calendar

Note You need to log in before you can comment on or make changes to this bug.