Closed
Bug 726279
Opened 13 years ago
Closed 13 years ago
Generalize use of Services.jsm in nsSearchService
Categories
(Firefox :: Search, enhancement)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 15
People
(Reporter: Yoric, Assigned: raymondlee)
References
Details
(Whiteboard: [mentor=Yoric][lang=js])
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
Yoric
:
review+
dao
:
review+
|
Details | Diff | Splinter Review |
Followup to bug 699856.
|
Reporter | |
Comment 1•13 years ago
|
||
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•13 years ago
|
Assignee: nobody → raymond
Status: NEW → ASSIGNED
|
|
Assignee | |
Comment 2•13 years ago
|
||
Attachment #619017 -
Flags: review?(dteller)
|
||
Comment 3•13 years ago
|
||
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•13 years ago
|
||
Removed _dirSvc, sbs and ss.
Attachment #619017 -
Attachment is obsolete: true
Attachment #619017 -
Flags: review?(dteller)
Attachment #619046 -
Flags: review?(dteller)
|
|
Reporter | |
Comment 5•13 years ago
|
||
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 6•13 years ago
|
||
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•13 years ago
|
|
||
Comment 7•13 years ago
|
||
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
|
|
||
Comment 8•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•