The default bug view has changed. See this FAQ.

Generalize use of Services.jsm in nsSearchService

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Search
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Yoric, Assigned: raymondlee)

Tracking

unspecified
Firefox 15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=Yoric][lang=js])

Attachments

(1 attachment, 1 obsolete attachment)

Followup to bug 699856.
nsSearchService.js was written before Services.jsm . Now that we have Services.jsm, we can simplify code in a few places.
Whiteboard: [mentor=Yoric][lang=js]
(Assignee)

Updated

5 years ago
Assignee: nobody → raymond
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 619017 [details] [diff] [review]
v1
Attachment #619017 - Flags: review?(dteller)
Comment on attachment 619017 [details] [diff] [review]
v1

> let _dirSvc = null;
> function getDir(aKey, aIFace) {
>   if (!aKey)
>     FAIL("getDir requires a directory key!");
> 
>   if (!_dirSvc)
>-    _dirSvc = Cc["@mozilla.org/file/directory_service;1"].
>-               getService(Ci.nsIProperties);
>+    _dirSvc = Services.dirsvc;
>   return _dirSvc.get(aKey, aIFace || Ci.nsIFile);
> }

>+      var sbs = Services.strings;
> 
>       var brandBundle = sbs.createBundle(BRAND_BUNDLE);
>       var brandName = brandBundle.GetStringFromName("brandShortName");
> 
>       var searchBundle = sbs.createBundle(SEARCH_BUNDLE);

>+      var ss = Services.search;
>       if (ss.getEngineByName(aEngine.name)) {
>         if (aEngine._confirm)
>           onError("error_duplicate_engine_msg", "error_invalid_engine_title");
> 
>         LOG("_onLoad: duplicate engine found, bailing");
>         return;
>       }

_dirSvc, sbs and ss are unnecessary.
(Assignee)

Comment 4

5 years ago
Created attachment 619046 [details] [diff] [review]
v2

Removed _dirSvc, sbs and ss.
Attachment #619017 - Attachment is obsolete: true
Attachment #619017 - Flags: review?(dteller)
Attachment #619046 - Flags: review?(dteller)
Comment on attachment 619046 [details] [diff] [review]
v2

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

Looks good to me. If this passes tests, you have my r+, but I defer to gavin for the final word.
Attachment #619046 - Flags: review?(gavin.sharp)
Attachment #619046 - Flags: review?(dteller)
Attachment #619046 - Flags: review+
Comment on attachment 619046 [details] [diff] [review]
v2

stealing... four reviewing eyes should be sufficient for this
Attachment #619046 - Flags: review?(gavin.sharp) → review+

Updated

5 years ago
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/integration/mozilla-inbound/rev/6606ae2e48d4

Also, to make life easier for those checking in patches on your behalf, please follow the directions below for future patches you submit. Thanks!
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → Firefox 15
http://hg.mozilla.org/mozilla-central/rev/6606ae2e48d4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.