451578 - Replace |.getService() .QueryInterface(iid)| by |.getService(iid)| in mozilla-central

Status: NEW

(Core :: XPCOM, defect)

Description

[Serge Gautherie (:sgautherie)]

Example:
{{
-          strBundleService =
-          Components.classes["@mozilla.org/intl/stringbundle;1"].getService(); 
-          strBundleService = 
-          strBundleService.QueryInterface(Components.interfaces.nsIStringBundleService);

+      gStrBundleService =
+        Components.classes["@mozilla.org/intl/stringbundle;1"]
+                  .getService(Components.interfaces.nsIStringBundleService);
}}

Comment 1

[:aceman]

What is the gain from this change? Does it have universal support among the developers?
There is potentially quite a lot of this pattern.
http://mxr.mozilla.org/comm-central/search?string=.getService%28%29&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to :aceman from comment #1)
> What is the gain from this change?

"Optimized" code: the result is the same.

> Does it have universal support among the developers?

It should. You can try it on a few files...

Comment 3

[:aceman]

OK, let's try it.

Comment 4

[:aceman]

On the other hand, many of them will go away just by converting to Services.jsm so I'd rather not do that now on whole Core and fixing the spots without converting to Services would probably pass through the reviewers.
So I need to find which spots are worth to do now.

Comment 5

[:aceman]

...whouldn't pass...

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to :aceman from comment #4)
> many of them will go away just by converting to Services.jsm

Good point ;-)

Comment 7

[:aceman]

I am not going to do this globally in Core as I can not test it properly (only building TB).

Comment 8

[Amod [:AMoz]]

Sir, i would like to contribute to this bug if it is not assigned to anyone. In that case, will you explain the bug in detail ?

Comment 9

[sakshi]

Hello, 

I would like to fix this bug.

Comment 10

[David Teller [:Yoric] - still alive but not very active]

Since sgautherie doesn't seem to be responding, I'll take over that mentoring.

Comment 11

[Amod [:AMoz]]

Hello Yoric, 
Felt extremely happy to see the bug active. I had commented about a year ago. I would like to work on it.
Two things I would like to mention:
\1. Within few hours I would be boarding a flight to return from SUMMIT 2013. So it might take 18-20 hours for me to return to this bug.
\2. This bug requires Find-and-replace strategy. Instead of doing it manually for all 91 instances, I would like to write a simple script or a java program that would automatically replace all the instances in all the files of mozilla-central.

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Automatically detecting the instances of .getService() is a good idea (although dxr can already help you there). As mentioned in comment 4, wherever this is applicable, it would be better to convert the code to use Services.jsm instead of calling into xpcom directly, so it might not be possible to write a simple script to cover these cases, though.

Comment 13

[Amod [:AMoz]]

Attachment: bug-451578-fix

From what I could interpret, I need to import < Services.jsm > in those files where < .getServices() > is used. 
Hence I have done changes only in 2 files and if I am going correct, I will makes the changes in rest of them as well. 
Thanks !

Comment 14

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 815040 [details] [diff] [review]
bug-451578-fix

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

Well, importing Services.jsm is not sufficient, you need to use it.
So, for instance, in browser_aboutHome.js, you should replace
 Cc["@mozilla.org/observer-service;1"]
       .getService(Ci.nsIObserverService)
with
 Services.obs

You can find the full list of services included in Services.jsm here: https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Services.jsm

Comment 15

[Amod [:AMoz]]

Attachment: sample_patchv2

Comment 16

[Amod [:AMoz]]

As per name, it is a sample patch. Once the approach is correct, I will make the changes to rest of instances. Also | nsICategoryManager | has no Service Accessor in Services.jsm

Comment 17

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 815065 [details] [diff] [review]
sample_patchv2

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

It's on the right way :)
Also, please mark your previous patches as obsolete when it's the case.

::: browser/base/content/browser.js
@@ +3237,5 @@
>    toOpenWindowByType("global:console", "chrome://global/content/console.xul");
>  }
>  
>  function BrowserDownloadsUI()
>  {

I believe that you forgot to import Services.jsm in this file.

Comment 18

[Amod [:AMoz]]

Attachment: patchv1

Repaired | .getService() | instances. 
Regarding | .QueryInterface() |, i got so many instances at 
http://mxr.mozilla.org/mozilla-central/search?string=QueryInterface%28%29&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

So thought of asking you before proceeding with it.

Comment 19

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 815452 [details] [diff] [review]
patchv1

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

Generally speaking, you should cut this in several smaller patches, to be able to test (and land) them progressively.
I haven't reviewed the whole patch. Generally, this looks like on the right track, with a few fixes detailed below.
Now, I'd like you to test and fix your patch before we proceed. Eventually, we might also decide to replace some calls to |Cu.import("...Services.jsm")| by |XPCOM.defineLazyModuleGetter|, on a case-by-case basis.

::: addon-sdk/source/lib/sdk/test/harness.js
@@ +22,5 @@
>  const unit = require("../deprecated/unit-test");
>  const test = require("../../test");
>  const url = require("../url");
>  
> +Cu.import("resource://gre/modules/Services.jsm");

Have you tested that this works?
Addon-sdk code behaves differently from everything I know.

@@ +28,2 @@
>  var cService = Cc['@mozilla.org/consoleservice;1'].getService()
>                 .QueryInterface(Ci.nsIConsoleService);

You could also replace this by Services.console, iirc.

::: build/bloatcycle.html
@@ +7,5 @@
>    From mozilla/toolkit/content
>    These files did not have a license
>  */
>  
> +Cu.import("resource://gre/modules/Services.jsm");

Have you tested this?

::: netwerk/test/httpserver/httpd.js
@@ +38,5 @@
>    "HttpServer",
>  ];
>  
>  Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> +Cu.import("resource://gre/modules/Services.jsm");

This probably won't work, as Cu is defined below.

::: netwerk/test/httpserver/test/head_utils.js
@@ +9,5 @@
>   * <runxpcshelltests.py>.
>   */
> + 
> +Cu.import("resource://gre/modules/Services.jsm");
> + 

Please don't add whitespaces.

::: netwerk/test/unit/test_cookiejars_safebrowsing.js
@@ +39,5 @@
>  var checkCookiePath = "/checkcookie";
>  var safebrowsingUpdatePath = "/safebrowsingUpdate";
>  var safebrowsingRekeyPath = "/safebrowsingRekey";
>  var httpserver;
>  

Have you imported Services.jsm?
Does this work?

::: netwerk/test/unit/test_unix_domain.js
@@ +17,5 @@
>  const threadManager = Cc["@mozilla.org/thread-manager;1"].getService();
>  
>  const allPermissions = parseInt("777", 8);
>  
> +Cu.import("resource://gre/modules/Services.jsm");

You are calling Services before importing it, that probably won't work.

::: services/sync/tps/extensions/mozmill/resource/stdlib/httpd.js
@@ +10,5 @@
>   * httpd.js.
>   */
>  
>  Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> +Cu.import("resource://gre/modules/Services.jsm");

Have you checked whether Cu is defined here?

::: toolkit/components/passwordmgr/test/test_bug_627616.html
@@ +4,5 @@
>    <title>Test bug 627616</title>
>    <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>        
>    <script type="text/javascript" src="prompt_common.js"></script>
>    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
> +  Cu.import("resource://gre/modules/Services.jsm");

I strongly doubt that this can work.

Comment 20

[Anup Kumar [:reznord]]

Hi,

I am interested in working on this bug. So, please can you assign this bug to me.

Thanks in advance,

Regards,
A.Anup

Comment 21

[David Teller [:Yoric] - still alive but not very active]

Anup, that depends. Amod, are you still working on this?

Comment 22

[Anup Kumar [:reznord]]

No, I have just started working on this bug. OK so, if that is the case I will stop it now and see for one more bug.

Sir, when will you be available in IRC?

Comment 23

[Amod [:AMoz]]

I am working on this bug.. I will resume the work within a day or two.
I will self-assign the bug to avoid any confusion.

Comment 24

[David Teller [:Yoric] - still alive but not very active]

Any news of that bug, Amod?

Comment 25

[:aceman]

I'll split comm-central work into a separate bug as it can be worked on in parallel.

Comment 26

[Amod [:AMoz]]

I will resume the bug within a while.
Sorry for delay.

Comment 27

[Amod [:AMoz]]

Sorry Yoric, Since I am busy with my academics, I wont be able to find time before March 2, 2014. If you feel it is too long to wait, you are free to assign it to someone more deserving. 
Thank you. Looking forward to work with you again after I return.

Comment 28

[Amod [:AMoz]]

I am back from the break. So I would be working on this bug now.

Comment 29

[David Teller [:Yoric] - still alive but not very active]

Any news, Amod?

Comment 30

[Amod [:AMoz]]

yes. Sorry for the delay. I will resume the work within a day.

Comment 31

[Amod [:AMoz]]

Since the bug is split, What steps do I need to take now ?

Comment 32

[:aceman]

Your current patch does not contain anything for comm-central (Thunderbird and Seamonkey) so you can continue where you left off without any changes. The patch only contains files for mozilla-central so if you do any more changes just keep them inside the mozilla-central tree.
You probably just need to address problems from comment 19.

Comment 33

[Amod [:AMoz]]

Attachment: patchv1_to_look_for_.rej_files

I have returned to this bug after a very long time. So while importing the patch, I found many .rej files. So I am uploading the patch just to locate those unsaved changes. Hence I am NOT ASKING FOR REVIEW.

Comment 34

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8408396 [details] [diff] [review]
patchv1_to_look_for_.rej_files

Well, if you're not asking for review, please don't set the flag that asks for review.

Comment 35

[Amod [:AMoz]]

I dint ask for the review in the comment where I mentioned I want to check for the .rej files. When I had done with it, I asked for your review.
Please accept my apologies in case of any misunderstanding. :)

Comment 36

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8408396 [details] [diff] [review]
patchv1_to_look_for_.rej_files

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

There is progress, thanks.

Since your patch is hitting lots of code, it would be useful if you could divide it in several subpatches, one per toplevel directory:
i.e. one for netwerk/*, one for browser/*, etc. This will simplify reviewing, testing and landing.

Also, please run the tests. I don't want to spend time reviewing patches that simply have no chance of working.

::: addon-sdk/source/lib/sdk/test/harness.js
@@ +29,5 @@
>    resolve();
>    return promise;
>  }
>  
> +var cService = Services.console;

This module doesn't import Services, so you can't use it. Generally, don't attempt to use Services in the addon-sdk, it uses a different module system.

Also, please test your patches, I'm almost sure that this would have shown up in a test.

::: build/bloatcycle.html
@@ +21,5 @@
>  }
>  
>  function canQuitApplication()
>  {
> +  var os = Services.obs;

This file doesn't have access to Services. Anyway, please don't touch files in build/.

::: dom/tests/unit/test_geolocation_provider.js
@@ +3,5 @@
>  const Cu = Components.utils;
>  const Cr = Components.results;
>  
>  Cu.import("resource://testing-common/httpd.js");
> +Cu.import("resource://gre/modules/Services.jsm");

What's the point of importing Services if you don't use it?

::: netwerk/test/httpserver/httpd.js
@@ +47,5 @@
>  const CC = Components.Constructor;
>  
>  const PR_UINT32_MAX = Math.pow(2, 32) - 1;
>  
> +Cu.import("resource://gre/modules/Services.jsm");

Please add a second argument |this| to this import. Theoretically, we should do it everywhere, for compatibility with B2G.

::: netwerk/test/httpserver/test/head_utils.js
@@ +8,5 @@
>   * Loads _HTTPD_JS_PATH file, which is dynamically defined by
>   * <runxpcshelltests.py>.
>   */
> +
> +Cu.import("resource://gre/modules/Services.jsm");

, this

::: netwerk/test/unit/test_authpromptwrapper.js
@@ +7,5 @@
>  
>  const nsIAuthInformation = Components.interfaces.nsIAuthInformation;
>  const nsIAuthPromptAdapterFactory = Components.interfaces.nsIAuthPromptAdapterFactory;
>  
> +Cu.import("resource://gre/modules/Services.jsm");

, this

::: netwerk/test/unit/test_unix_domain.js
@@ +2,5 @@
>  
>  const CC = Components.Constructor;
> +const Cu = Components.utils;
> +
> +Cu.import("resource://gre/modules/Services.jsm");

, this

::: security/manager/pki/resources/content/resetpassword.js
@@ +5,5 @@
>  const nsPK11TokenDB = "@mozilla.org/security/pk11tokendb;1";
>  const nsIPK11TokenDB = Components.interfaces.nsIPK11TokenDB;
>  const nsIDialogParamBlock = Components.interfaces.nsIDialogParamBlock;
>  
> +Cu.import("resource://gre/modules/Services.jsm");

, this

::: services/sync/tps/extensions/mozmill/resource/stdlib/httpd.js
@@ +27,5 @@
>  const CC = Components.Constructor;
>  
>  const PR_UINT32_MAX = Math.pow(2, 32) - 1;
>  
> +Cu.import("resource://gre/modules/Services.jsm");

, this

::: testing/mochitest/server.js
@@ +10,5 @@
>  
>  // Disable automatic network detection, so tests work correctly when
>  // not connected to a network.
> +
> +Cu.import("resource://gre/modules/Services.jsm");

Let's not introduce dependencies to Services in this directory.

::: testing/xpcshell/head.js
@@ +9,5 @@
>   * See http://developer.mozilla.org/en/docs/Writing_xpcshell-based_unit_tests
>   * for more information.
>   */
>  
> +Cu.import("resource://gre/modules/Services.jsm");

Nor here.

::: toolkit/components/help/content/contextHelp.js
@@ +1,5 @@
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> +Cu.import("resource://gre/modules/Services.jsm");

, this

::: toolkit/components/passwordmgr/test/test_bug_627616.html
@@ +31,5 @@
>        }
>      });
>  
>      function init1() {
> +        var ios = Services.io;

Services is not defined. This would have showed up in a test.

::: toolkit/components/passwordmgr/test/test_prompt.html
@@ +6,5 @@
>    <script type="text/javascript" src="pwmgr_common.js"></script>
>    <script type="text/javascript" src="prompt_common.js"></script>
>    <script type="text/javascript" src="notification_common.js"></script>
>    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
> +  Cu.import("resource://gre/modules/Services.jsm");

That's not even syntactically correct. Please run the tests.
I'll stop my review here because the rest seems also syntactically incorrect.

Comment 37

[Amod [:AMoz]]

Sorry Yoric for giving no updates for a month. And concerning my current academic endeavors, I wont get time for another 1 month. So I think it would be better to leave the bug. 
Thank you for helping me !

Comment 38

[Anup Kumar [:reznord]]

Hi, 

I would like to work on this bug. So please can you assign it to me?

Thanks in advance,

Regards,
Anup

Comment 39

[Anup Kumar [:reznord]]

Attachment: Fix for /addon-sdk/* [Checked in: Comment 45]

Changes made as requested in the whole mozilla-central (only a few are to be changed.) Most of them are already changed in the latest revisions of the code.

Comment 40

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8450367 [details] [diff] [review]
Fix for /addon-sdk/*
[Checked in: Comment 45]

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

r=me if this passes all tests

Comment 41

[Anup Kumar [:reznord]]

Attachment: Link for a try request

In this I am attaching the try request link which I pulled today.

Comment 42

[David Teller [:Yoric] - still alive but not very active]

Reposting in the body.
Try: https://tbpl.mozilla.org/?tree=Try&rev=1ac67f7d7fd6

Comment 43

[David Teller [:Yoric] - still alive but not very active]

That looks good. Do you have the rights to mark your bug as checkin-needed?

Comment 44

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fda29051ad88

Comment 45

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/fda29051ad88

Comment 46

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/47eac113a15a51e6c805d17ab6cbbf616c9c5d60
Bug 451578 - Replace |.getService() .QueryInterface(iid)| by |.getService(iid)| in mozilla-central. r=Yoric

Comment 47

[Serge Gautherie (:sgautherie)]

(In reply to Tim Taubert [:ttaubert] from comment #45)
> Status: NEW → RESOLVED

I don't think so : this patch fixed 2 instances only, "many" of them remain.

Comment 48

[Anup Kumar [:reznord]]

(In reply to Serge Gautherie (:sgautherie) from comment #47)
> (In reply to Tim Taubert [:ttaubert] from comment #45)
> > Status: NEW → RESOLVED
> 
> I don't think so : this patch fixed 2 instances only, "many" of them remain.

I think the patch which I've submitted earlier is correct because according to the bug we need to replace *.getService() .QueryInterface(iid)* with *.getService()*. The search which I've used for is http://mxr.mozilla.org/mozilla-central/search?string=.getService%28%29%2B.QueryInterface%28iid%29

Correct me if I am wrong.

Comment 49

[:aceman]

(In reply to Anup Kumar from comment #48)
> (In reply to Serge Gautherie (:sgautherie) from comment #47)
> > (In reply to Tim Taubert [:ttaubert] from comment #45)
> > > Status: NEW → RESOLVED
> > 
> > I don't think so : this patch fixed 2 instances only, "many" of them remain.
> 
> I think the patch which I've submitted earlier is correct because according
> to the bug we need to replace *.getService() .QueryInterface(iid)* with
> *.getService()*. The search which I've used for is
> http://mxr.mozilla.org/mozilla-central/search?string=.getService%28%29%2B.
> QueryInterface%28iid%29
> 
> Correct me if I am wrong.
You cannot search for the fixed string "iid" because there can be anything in the .QueryInterface() call. Also, the .getService().QueryInterface() call chain is often split into multiple lines and that is probably not found easily with such a search.

Also, there are cases like:
http://mxr.mozilla.org/mozilla-central/source/dom/tests/unit/test_geolocation_provider.js#86

So I think you should just search for ".getService()" (as I've done in comment 1) and manually review all hits.

Comment 50

[Anup Kumar [:reznord]]

(In reply to :aceman from comment #49)
> Also, there are cases like:
> http://mxr.mozilla.org/mozilla-central/source/dom/tests/unit/
> test_geolocation_provider.js#86
> 
> So I think you should just search for ".getService()" (as I've done in
> comment 1) and manually review all hits.

Thanks for correcting me. I forgot about the iid part of the search index. Coming to the search for *.getService()* this is the correct search right? 
http://dxr.mozilla.org/mozilla-central/search?q=getService()+ext%3Ajs&case=true

Comment 51

[Serge Gautherie (:sgautherie)]

(In reply to Anup Kumar from comment #50)
> Coming to the search for *.getService()* this is the correct search right? 
> http://dxr.mozilla.org/mozilla-central/
> search?q=getService()+ext%3Ajs&case=true

JS files are most of the cases, but not all of them.
I'm not sure whether the search should be made with the beginning '.' or not, to catch all relevant cases...

Comment 52

[Anup Kumar [:reznord]]

Attachment: Replaced all the .getService.QuertInterface( ) in js files too.

Made all the changes in the tests also including all the js files.

Comment 53

[Serge Gautherie (:sgautherie)]

Comment on attachment 8490789 [details] [diff] [review]
Replaced all the .getService.QuertInterface( ) in js files too.

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

I had a quick look: these changes look good ;-)

::: toolkit/components/printing/content/printPageSetup.js
@@ -256,5 @@
>    var print_margin_left   = 0.5;
>    var print_margin_bottom = 0.5;
>    var print_margin_right  = 0.5;
>  
>    try {

If the following 'if's need to stay, maybe add a comment(s) to explain why.

@@ -260,5 @@
>    try {
>      gPrefs = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);
>    
> -    gPrintService = Components.classes["@mozilla.org/gfx/printsettings-service;1"];
> -    if (gPrintService) {

I think you can't merge this 'if', because (iirc) the class may not be available if there is no printer. (To be confirmed.)

@@ -262,5 @@
>    
> -    gPrintService = Components.classes["@mozilla.org/gfx/printsettings-service;1"];
> -    if (gPrintService) {
> -      gPrintService = gPrintService.getService();
> -      if (gPrintService) {

I assume you can indeed merge this 'if'. (To be confirmed.)

::: toolkit/components/printing/content/printdialog.js
@@ -217,5 @@
>    var print_frametype     = gPrintSetInterface.kSelectedFrame;
>    var print_howToEnableUI = gPrintSetInterface.kFrameEnableNone;
>    var print_tofile        = "";
>  
>    try {

Same as 'printPageSetup.js'...

Comment 54

[Serge Gautherie (:sgautherie)]

Comment on attachment 8450367 [details] [diff] [review]
Fix for /addon-sdk/*
[Checked in: Comment 45]

Anup, don't obsolete checked in patches.

Comment 55

[:aceman]

"printing" can be disabled at compile time. If that removes this exact service, I don't know. But better do not remove the ifs in such code patterns.

Comment 56

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8490789 [details] [diff] [review]
Replaced all the .getService.QuertInterface( ) in js files too.

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

The patch looks good to me.
If NS_PRINTING is undefined, `Components.classes["@mozilla.org/gfx/printsettings-service;1"]` will return `undefined` and the rest will be caught by the `try/catch`. Otherwise, as far as I can tell, the only bad thing that can happen is an error while instantiating the service, which will also cause an exception which will also be caught.

In other words, if the tests pass, I'm satisfied. Anup, can you run the tests on all platforms to make sure that everything is alright?

Comment 57

[Anup Kumar [:reznord]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #56)
> In other words, if the tests pass, I'm satisfied. Anup, can you run the
> tests on all platforms to make sure that everything is alright?

Hi Yoric,

I am sorry to say that I can't put a try request now because as I told you earlier I lost my public key. I need to file a bug for that and it might take a while. So please can you put the try request for me.

Regards,
Anup

Comment 58

[David Teller [:Yoric] - still alive but not very active]

Try: https://tbpl.mozilla.org/?tree=Try&rev=737a14279d60

Comment 59

[David Teller [:Yoric] - still alive but not very active]

Try results have been wiped a long time ago.
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=35d577bb7408

Comment 60

[Stanislas Daniel Claude Dolcini]

I looked for all those functions in the mozilla-central codebase. I replaced the first case  in server.js

@@ -49,8 +49,7 @@
   httpserv.registerPathHandler("/bug482601/only_from_cache", bug482601_only_from_cache);
   httpserv.start(-1);
 
-  var obs = Cc["@mozilla.org/observer-service;1"].getService();
-  obs = obs.QueryInterface(Ci.nsIObserverService);
+  var obs = Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);
   obs.addObserver(observer, "http-on-examine-response", false);
   obs.addObserver(observer, "http-on-examine-merged-response", false);
   obs.addObserver(observer, "http-on-examine-cached-response", false);

There is another file IIRC but it requires a complete rewrite of the code used.

Can I be assigned to this ticket ?

Comment 61

[Stanislas Daniel Claude Dolcini]

Attachment: Replace the getservice.queryinterface.

Comment 62

[Serge Gautherie (:sgautherie)]

(In reply to Stanislas Daniel Claude Dolcini from comment #60)

> I looked for all those functions in the mozilla-central codebase.

Compared to previous patch,
your patch misses 'test_net_failedtoprocess.html', at least.

> +  var obs =
> Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);

Did you try to do the replacements as in comment 14, where applicable?
(Please, read (again) all previous bug comments for added details about this bug.)

> Can I be assigned to this ticket ?

Done :-)

Comment 63

[Stanislas Daniel Claude Dolcini]

Hello, thanks for the fast reply.

Actually I looked into js files, that's why I missed the html one(s)
> Did you try to do the replacements as in comment 14, where applicable? (Please, read (again) all previous bug comments for added details about this bug.)
Will do.

> Done :-)
Thanks !

Comment 64

[Stanislas Daniel Claude Dolcini]

Attachment: bug_451578.patch

This adds the Services.Jsm to all the files so that we can use lazy shortcuts.

There is one missing PrintingPrompt.

Comment 65

[Serge Gautherie (:sgautherie)]

Comment on attachment 8674913 [details] [diff] [review]
bug_451578.patch

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

(Sorry for the long delay.)

This seems promising.
Please add a link to a (successful) Try run.

> There is one missing PrintingPrompt

I'm not sure what you meant.

::: dom/tests/unit/test_geolocation_provider.js
@@ +84,4 @@
>      prefs.setBoolPref("dom.testing.ignore_ipc_principal", true);
>      prefs.setBoolPref("geo.wifi.scan", false);
>  
> +	var obs = Services.obs;

Nit: check that you are not adding more indentation than needed.

Comment 66

[Matthew N. [:MattN]]

Unassigning due to inactivity in case someone wants to finish this.

Comment 67

[Videet Singhai]

Hey, can anyone tell me the current status of this issue?

Comment 68

[Videet Singhai]

Hey, I am trying to contribute to this bug. How to build the code ( a way to check if everything works fine?)?
Thank you.

Comment 69

[:aceman]

Hi, thanks for the interest. You can try the build instructions from here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Thunderbird_build .

Comment 70

[Amit]

Hi,I wanted to know the status of the bug. Is it still active? I want to take it up as my first big
Thank You

Comment 71

[:aceman]

Looks like it has stalled. You can take it if you wish.

Comment 72

[Mihir Karbelkar(:fauxwizard_)]

Hey is it still open, I would like to take it as my first bug. If not can you recommend where should I go?

Comment 73

[:aceman]

Yes, it seems you can take it. There is already an existing patch (2 of them) that need to be finished.

Comment 74

[akanksha bhardwaj]

Hello! Is someone still working on it?, if not I would like to be assigned.

Comment 75

[Mihir Karbelkar(:fauxwizard_)]

Hey! Akanksha bhardwaj you can take it if you want I was going to work on this but my laptop stopped working recently and also I have already done my first bug so if you are just starting it will be good for you. Thanks!

Comment 76

[Jeff Walden [:Waldo]]

This is one of those catch-all sorts of bugs that's a whole-codebase concern.  It definitely is not a Core::JavaScript Engine concern.  Core::XPCOM because this is XPCOM use and I suppose I ought to avoid Core::General.

Comment 77

[Hamzah Akhtar]

Hey! I would to like to work on this bug. As this bug is my first, can someone guide so as to where should I start from.

Comment 78

[Mahak]

hey !!
Can I take this up ?
Thanks
mahak :)