Closed Bug 769737 Opened 8 years ago Closed 8 years ago

Add nsIFocusManager to Services.jsm.

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: philip.chee, Assigned: philip.chee)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 1 obsolete file)

Neil said to me:
>> +  var fm = Components.classes["@mozilla.org/focus-manager;1"]
>> +                     .getService(Components.interfaces.nsIFocusManager);
> [Surprised this isn't in Services yet ;-) ]
Attached patch Patch v1.0 add nsIFocusManager. (obsolete) — Splinter Review
> --- a/toolkit/content/Services.jsm
> +++ b/toolkit/content/Services.jsm

Services.jsm used to be in alphabetical order but apparently some people didn't get the memo. I've resorted this file.

>    ["ww", "@mozilla.org/embedcomp/window-watcher;1", "nsIWindowWatcher"],
> -  ["startup", "@mozilla.org/toolkit/app-startup;1", "nsIAppStartup"],

Keeping a trailing comma should make blame clearer.

From http://ecma-international.org/ecma-262/5.1/#sec-D

11.1.4: Edition 5 clarifies the fact that a trailing comma at the end of an ArrayInitialiser  does not add to the length of the array. This is not a semantic change from Edition 3 but some implementations may have previously misinterpreted this.

>    checkService("clipboard", Ci.nsIClipboard);
> +  checkService("appShell", Ci.nsIAppShellService);
> +  checkService("cache", Ci.nsICacheService);
> +  checkService("scriptSecurityManager", Ci.nsIScriptSecurityManager);
> +  checkService("domStorageManager", Ci.nsIDOMStorageManager);
> +  checkService("DOMRequest", Ci.nsIDOMRequestService);
> +  checkService("downloads", Ci.nsIDownloadManager);
> +  checkService("focus", Ci.nsIFocusManager);

Added missing tests. I could not discern any order in this file except historical order, so I've added these tests in the order that they were added to Services.jsm.

$ MOZ_NO_REMOTE=1 TEST_PATH=toolkit/content/tests/browser/browser_Services.js pymake -C ../objdir/ mochitest-browser-chrome

INFO TEST-START | Shutdown
Browser Chrome Test Summary
        Passed: 77
        Failed: 0
        Todo: 0
Attachment #637951 - Flags: review?(gavin.sharp)
Comment on attachment 637951 [details] [diff] [review]
Patch v1.0 add nsIFocusManager.

I don't think there's any value in keeping the list sorted alphabetically (not like it's hard to search), can you attach a patch that omits those changes?
Attachment #637951 - Flags: review?(gavin.sharp)
> I don't think there's any value in keeping the list sorted alphabetically (not
> like it's hard to search), can you attach a patch that omits those changes?
Attached herewith.
Attachment #637951 - Attachment is obsolete: true
Attachment #638116 - Flags: review?(gavin.sharp)
Comment on attachment 638116 [details] [diff] [review]
Patch v1.1 unsorted.

Thanks!
Attachment #638116 - Flags: review?(gavin.sharp) → review+
Pushed to inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/4c793a367b2f
Target Milestone: --- → mozilla18
http://hg.mozilla.org/mozilla-central/rev/4c793a367b2f
Target Milestone: mozilla18 → mozilla16
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.