181442 - Switch composer off of the deprecated nsIPref and start using nsIPrefBranch and friends

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Proposed patch

Comment 2

[Kathleen :Brade]

Comment on attachment 107140 [details] [diff] [review]
Proposed patch

Looks great; here's some little cleanup that shouldn't be too difficult.

change this to the preferred assignment style:
>Index: editor/composer/src/nsEditorSpellCheck.cpp
>+  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(kPrefServiceCID, &rv));
should be:
 +  nsCOMPtr<nsIPrefBranch> prefBranch = do_GetService(kPrefServiceCID, &rv);

same change here:
>+  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(kPrefServiceCID, &rv));


I'd like to see some consistency for the do_GetService calls in this patch if
possible.  In nsEditorSpellCheck.cpp and other places you use kPrefServiceCID
and define a static in the file but elsewhere (as in nsEditor.cpp and
nsTextEditRules.cpp) you use NS_PREFSERVICE_CONTRACTID.  Am I missing something
subtle?

>+  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(kPrefServiceCID, &rv));
vs.
+  nsCOMPtr<nsIPrefBranch> prefBranch =
+    do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);

See also:
>Index: editor/libeditor/html/nsHTMLCSSUtils.cpp
>Index: editor/libeditor/html/nsHTMLDataTransfer.cpp
>Index: editor/libeditor/html/nsHTMLEditRules.cpp
>Index: editor/libeditor/text/nsEditorEventListeners.cpp
>Index: editor/libeditor/text/nsPlaintextEditor.cpp


use assignment form here:
>Index: editor/libeditor/html/nsHTMLEditRules.cpp
>+  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(kPrefServiceCID, &res));

and here:
>Index: editor/libeditor/text/nsEditorEventListeners.cpp
>+    nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(kPrefServiceCID, &rv));

and here:
>Index: editor/libeditor/text/nsPlaintextEditor.cpp
>+    nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(kPrefServiceCID, &rv));
>+  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(kPrefServiceCID, &rv));

and here:
>Index: editor/libeditor/text/nsTextEditRules.cpp
>+  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));

Comment 3

[kinmoz]

Personally I prefer if we go with use of the NS_PREFSERVICE_CONTRACTID so we can
get rid of all the NS_DEFINE_CID declarations in each of our files that use prefs.

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v2

The CIDs are gone, and I used the = operator for ctors instead.

Comment 5

[Kathleen :Brade]

Comment on attachment 107177 [details] [diff] [review]
Patch v2

HURRAY!  r=brade

Comment 6

[kinmoz]

Comment on attachment 107177 [details] [diff] [review]
Patch v2

sr=[e-mail]

Thanks for doing this!

Comment 7

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked in.