1110109 - Use Services.focus instead of Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager) in browser/base/

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

Services.focus should be faster and is prettier than the XPCOM getService calls.

Affected files:

http://mxr.mozilla.org/mozilla-central/search?string=nsifocusman&find=%2Fbrowser%2Fbase%2F

Comment 1

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 1 - Using Services.focus instead of Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager)

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8536519 [details] [diff] [review]
rev 1 - Using Services.focus instead of Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager)

>--- a/browser/base/content/browser-tabPreviews.js
>+++ b/browser/base/content/browser-tabPreviews.js
>@@ -128,18 +130,17 @@ var tabPreviewPanelHelper = {
>     if ("setupGUI" in host)
>       host.setupGUI();
>   },
>   _popuphiding: function (host) {
>     if ("suspendGUI" in host)
>       host.suspendGUI();
> 
>     if (host._prevFocus) {
>-      Cc["@mozilla.org/focus-manager;1"]
>-        .getService(Ci.nsIFocusManager)
>+      Services.focus
>         .setFocus(host._prevFocus, Ci.nsIFocusManager.FLAG_NOSCROLL);

nit: the last line should be indented further such that ".focus" and ".setFocus" line up.

This patch basically looks good, but please remove the calls to Components.utils.import. They are redundant here, utilityOverlay.js already has you covered: http://hg.mozilla.org/mozilla-central/annotate/5d6e0d038f95/browser/base/content/utilityOverlay.js#l7

Comment 3

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 2 - Using Services.focus instead of Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager)

Changes made after updating mozilla-central

Comment 4

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [:dao] from comment #2)
> but please remove the calls to Components.utils.import.

These are still there in your new patch. Seems like you misunderstand my comment?

Also, can you please drop the utilityOverlay.js changes? They're off-topic for this bug.

Comment 5

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 3 - Using Services.focus instead of Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager)

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8537364 [details] [diff] [review]
rev 3 - Using Services.focus instead of Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager)

Thanks!

Comment 7

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/1857b132f618

Comment 8

[Dão Gottwald [:dao]]

Abdelrhman, if you're interested in fixing slightly more involved bugs, you might want to take a look at bug 1112552 or bug 1112556 that I just filed.

Comment 9

[Dão Gottwald [:dao]]

Again, it looks like this bug wasn't updated when the patch was merged to mozilla-central.

https://hg.mozilla.org/mozilla-central/rev/1857b132f618