+++ This bug was initially created as a clone of Bug #1433175 +++ Bug 767640, comment 32: "Mass replace the remaining Components.interface/Components.utils uses with Ci/Cu (I'm hesitant about Components.classes -> Cc because I'm not sure we have a standard way to deal with the indent). And after doing this, covering it with eslint would make sense." === The latest push shows heaps of linting errors: Use Ci rather than Components.interfaces Use Cc rather than Components.classes Use Cr rather than Components.results (I didn't see the Cu equivalent). So this looks like a massive find/replace job.

The rule was introduced in bug 1230369.

This disables the rule for now, looking at the code and bug 1433175 it seems this is not yet ready on the m-c side. I'm hoping they produce some sort of script we can run to mass replace.

Comment on attachment 8949313 [details] [diff] [review] Disable the rule - v1 [landed comment #4] Thanks, I'll land this now with some full stops at the end of the comments ;-)

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/a31a998b9f30 Disable eslint mozilla/use-cc-etc rule for the time being. r=jorgk

This is the result of running my CcCiCuCr.js script written for bug 1433175 on comm-central. This script was meant to be safe, so it should cause no other damage than breaking the indent of a few lines in rare cases.

This is a second script generated patch applying on top of the previous one, created by the inCcCiCuCr.js script, meant to be much more aggressive to eliminate all the remaining cases. This is not safe, it needs careful review. And this probably doesn't make any sense for im/ or suite/ without first applying something like bug 1436310.

Comment on attachment 8954858 [details] [diff] [review] more aggressive patch Review of attachment 8954858 [details] [diff] [review]: ----------------------------------------------------------------- Looks good apart from some problems I flagged. ::: im/components/contentHandler.js @@ +5,5 @@ > ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); > > +var Ci = Ci; > +var Cc = Cc; > +var Cr = Cr; Yep, these need to go. ::: im/components/profileMigrator.js @@ +3,5 @@ > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); > +var Cc = Cc; > +var Ci = Ci; And here. ::: im/content/blist.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/. */ > > +var Cu = Cu; And here. ::: im/content/engineManager.js @@ +2,5 @@ > * 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/. */ > > +var Ci = Ci; > +var Cc = Cc; And here. ::: im/content/preferences/applicationManager.js @@ +3,5 @@ > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > #ifdef XP_MACOSX > +var Cc = Cc; > +var Ci = Ci; And here and a few more times further down. ::: mail/test/resources/mozmill/mozmill/extension/resource/modules/frame.js @@ +104,5 @@ > module.elementslib = elementslib; > module.persisted = persisted; > + module.Cc = Cc; > + module.Ci = Ci; > + module.Cu = Cu; What to do with this? @@ +114,5 @@ > elementslib: elementslib, > persisted: persisted, > + Cc: Cc, > + Ci: Ci, > + Cu: Cu } And this? ::: mail/test/resources/mozmill/mozmill/extension/resource/stdlib/securable-module.js @@ +194,5 @@ > if (module == "chrome") { > + var chrome = { Cc: Cc, > + Ci: Ci, > + Cu: Cu, > + Cr: Cr, More of the same. ::: suite/browser/navigator.js @@ +2541,5 @@ > return null; > > try { > // are we a popup window? > + const CI = Ci; Replace with Ci. @@ +2639,5 @@ > * or frameset. > */ > function getCurrentURI() > { > + const CI = Ci; And here. @@ +2651,5 @@ > } > > function uploadFile(fileURL) > { > + const CI = Ci; And here and more further down.

How should we proceed here? Do a try run on the first part and then land it. This is highly susceptible to rot.

Try with part 1 here: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ad577476f5774aff1eb3113bd0c4beb8ef5255e7 I might just go ahead and land this to avoid rot. I did some random checks and they looked good. Pity we have test bustage today (bug 1441812) so the try result won't be all that conclusive.

Comment on attachment 8954854 [details] [diff] [review] script generated patch [landed comment #16] I did a brief skim and this looks ok. I have a boatload of patches in bug 905097 and dependencies awaiting review, and this patch would probably bitrot them all. Given it is script generated, maybe it makes sense to exclude calendar for now and run the script again once I have the calendar patches reviewed. In addition, for the gdata provider we'll need to add backwards compat to make sure Ci/Cc/Cr is defined even if installed in an older version.

Comment on attachment 8954858 [details] [diff] [review] more aggressive patch Review of attachment 8954858 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/test/resources/mozmill/mozmill/extension/resource/modules/frame.js @@ +104,5 @@ > module.elementslib = elementslib; > module.persisted = persisted; > + module.Cc = Cc; > + module.Ci = Ci; > + module.Cu = Cu; We need to keep these, as they are defining the globals in a sandbox. @@ +114,5 @@ > elementslib: elementslib, > persisted: persisted, > + Cc: Cc, > + Ci: Ci, > + Cu: Cu } Same here ::: mail/test/resources/mozmill/mozmill/extension/resource/stdlib/securable-module.js @@ +194,5 @@ > if (module == "chrome") { > + var chrome = { Cc: Cc, > + Ci: Ci, > + Cu: Cu, > + Cr: Cr, This likely also needs to stay, but we could just use var chrome = { Cc, Ci, Cu, Cr, Cm: Components.manager, components: Components }

Here is a the cleaned version of the more aggressive patch. It is untested, I mainly removed the self-assignments. I did go through each hunk though.

Comment on attachment 8954937 [details] [diff] [review] more aggressive patch - v2 [landed comment #16] Review of attachment 8954937 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, thanks for going through all of this and cleaning up so many files by hand! ::: mail/test/resources/mozmill/mozmill/extension/resource/stdlib/securable-module.js @@ +192,5 @@ > var self = this; > return function require(module) { > if (module == "chrome") { > + var chrome = { > + Cc, Ci, Cu, Cr, nit: trailing whitespace. ::: mailnews/base/util/errUtils.js @@ +120,5 @@ > return str; > }, > > getStack: function(skipCount) { > + if (typeof Components != "object" || typeof Cc != "object") I have no idea of what this code is trying to do, neither before nor after the patch.

I wanted to land part 1 without the calendar part, but my try push still hasn't even started :-( Looking at the about 60.000 lines of patch after removing the calendar hunks, there is some pretty bad indentation where "Ci" ends up at the end of the line. Look for Ci$ in the patch and you'll find beauties like: + QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, + Ci + .nsIAutoCompleteSearch, + Ci + .nsIAbDirSearchListener]) or + let migrator = Cc[cid] + .createInstance(Ci + .nsISuiteProfileMigrator); I'll postpone landing this for another day.

Comment on attachment 8954937 [details] [diff] [review] more aggressive patch - v2 [landed comment #16] Uff, I read through the whole thing.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/f675a6fda965 script generated patch to replace the Components.interface/Components.utils uses with Ci/Cu in C-C (without calendar). r=philipp https://hg.mozilla.org/comm-central/rev/d32dfb61ec67 more aggressive patch to replace the remaining Components.interface/Components.utils uses with Ci/Cu in C-C (without calendar). r=jorgk

I am rebasing my patches right now and still see a lot of Components.interfaces outside of calendar in c-c. Is this intentional or does it need further cleanup? https://dxr.mozilla.org/comm-central/search?q=Components.interfaces.+path%3Amail%2F&redirect=false

Thanks for letting us know. I wouldn't call it "a lot", but there are some left, 17 according to the query :-( Looks like XUL and XML files and comments weren't treated. These comments https://dxr.mozilla.org/comm-central/search?q=Components.interfaces.+path%3Amailnews%2F&redirect=true could also get a clean-up.

> I wouldn't call it "a lot", but there are some left, Adds up when you add mailnews and suite but not a big problem. See a few Components.classes too: https://dxr.mozilla.org/comm-central/search?q=Components.+path%3Amail%2F&redirect=false Can Components.manager and Components.Constructor also be replaced?

(In reply to Jorg K (GMT+1) from comment #18) > Looks like XUL and XML files and comments weren't treated. In .xml/.xhtml/.xul files, the script only replaces code within CDATA sections. Putting code in these files outside of CDATA sections is not a good idea anyway ( & < > characters which are common in conditions would cause parse errors). (In reply to Frank-Rainer Grahl (:frg) from comment #19) > Can Components.manager and Components.Constructor also be replaced? No, only classes, interfaces, utils and results have shorthands exposed in all chrome scopes by default.

All Seamonkey hits I have found, untested.

Hits I found in Thunderbird (/mail, /mailnews, /chat, /common). Even found a bug (typo). Can you spot it? ;)

Comment on attachment 8958222 [details] [diff] [review] 1436605.patch - Thunderbird remainders Review of attachment 8958222 [details] [diff] [review]: ----------------------------------------------------------------- I assume the removed file is no longer needed. ::: mail/base/content/tabmail.xml @@ +1781,5 @@ > this.arrowScrollbox.removeEventListener("underflow", this); > ]]> > </destructor> > > + <field name="tabmail" readonly="true"><![CDATA[ Why add all those CDATAs? You didn't do it in mailWidgets.xml and preferences.xml. ::: mail/components/newmailaccount/content/uriListener.js @@ -39,5 @@ > aTopic != "http-on-examine-cached-response") > return; > > if (!(aSubject instanceof Ci.nsIHttpChannel)) { > - Component.utils.reportError("Failed to get a nsIHttpChannel when " Typo here, right? ::: mailnews/db/gloda/content/glodacomplete.xml @@ +43,1 @@ > wrappedJSObject; Please reformat the entire assignment to match the standard.

(In reply to Jorg K (GMT+1) from comment #23) > Comment on attachment 8958222 [details] [diff] [review] > 1436605.patch - Thunderbird remainders > > Review of attachment 8958222 [details] [diff] [review]: > ----------------------------------------------------------------- > > I assume the removed file is no longer needed. Yes, it seems already applied (the changes in the patch are in the EventUtils.js file). Looks like some very old temporary left-over. > ::: mail/base/content/tabmail.xml > @@ +1781,5 @@ > > this.arrowScrollbox.removeEventListener("underflow", this); > > ]]> > > </destructor> > > > > + <field name="tabmail" readonly="true"><![CDATA[ > > Why add all those CDATAs? You didn't do it in mailWidgets.xml and > preferences.xml. Yeah, I didn't want to expand it much further, only around the place I touched. Apparently missing CDATA blocks are wrong, maybe we should make a bug to add them everywhere. Should I drop the ones added here? > ::: mail/components/newmailaccount/content/uriListener.js > > if (!(aSubject instanceof Ci.nsIHttpChannel)) { > > - Component.utils.reportError("Failed to get a nsIHttpChannel when " > > Typo here, right? Right :) > ::: mailnews/db/gloda/content/glodacomplete.xml > @@ +43,1 @@ > > wrappedJSObject; > > Please reformat the entire assignment to match the standard. OK

Thanks.

(In reply to :aceman from comment #24) > > ::: mail/base/content/tabmail.xml > > @@ +1781,5 @@ > > > this.arrowScrollbox.removeEventListener("underflow", this); > > > ]]> > > > </destructor> > > > > > > + <field name="tabmail" readonly="true"><![CDATA[ > > > > Why add all those CDATAs? You didn't do it in mailWidgets.xml and > > preferences.xml. > > Yeah, I didn't want to expand it much further, only around the place I > touched. Apparently missing CDATA blocks are wrong, maybe we should make a > bug to add them everywhere. Should I drop the ones added here? These CDATA blocks are ugly, and probably not necessary when it's wrapping one or two simple lines. They are needed when the code in the block contains one of these characters: < > & This happens frequently when there's an if statement. So IMO putting a CDATA tag in <field name="_prefObserver"> makes sense. The others can probably be safely omitted.

OK.

Comment on attachment 8958256 [details] [diff] [review] 1436605.patch - Thunderbird remainders v1.2 Review of attachment 8958256 [details] [diff] [review]: ----------------------------------------------------------------- As per IRC. Also note: https://dxr.mozilla.org/comm-central/rev/59c0f6e4825c4db83eb64c4dbd6ed11aee7be98e/mail/base/content/search.xml#70 ::: mail/base/content/tabmail.xml @@ +1839,3 @@ > </field> > > + <field name="_tabDropIndicator"><![CDATA[ remove CDATA here as well. ::: mailnews/db/gloda/content/glodacomplete.xml @@ +38,5 @@ > // to change fast enough. > item = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "richlistitem"); > > + let glodaCompleter = Cc["@mozilla.org/autocomplete/search;1?name=gloda"] > + .getService() //Ci.nsIAutoCompleteSearch) Funny comment here.

Yes, going by other autocomplete objects, let's specify the Ci.nsIAutoCompleteSearch service explicitly instead of the comment. I've tested the gloda search textbox that this code is hit there and autocomplete results are still populated in the dropdown.

Comment on attachment 8958221 [details] [diff] [review] 1436605.patch - Seamonkey remainders [landed comment #34] Looks good. Big thanks. There are some minor inconsistencies with single and double quotes (which some I probably introduced myself) but I wouldn't change them here. A lot of the code here needs still to be touched for our ESR60 release to work (rdf datasources) so asking IanN for beta approval too. [Approval Request Comment] Regression caused by (bug #): -- User impact if declined: backporting bugs would be harder. Testing completed (on c-c, etc.): Risk to taking this patch (and alternatives if risky): none cosmetic.

Comment on attachment 8958221 [details] [diff] [review] 1436605.patch - Seamonkey remainders [landed comment #34] IanN: NI because the bug is in TB so you probably won't get a beta a? request for the suite part.

Comment on attachment 8958278 [details] [diff] [review] 1436605.patch - Thunderbird remainders v1.3 [landed comment #34] Thanks, I'll get that landed.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/4b2c136f620a Replace remaining Components.interface/Components.utils uses with Ci/Cu in mail, chat, common and mailnews. r=jorgk https://hg.mozilla.org/comm-central/rev/8853a3eec03c Replace remaining Components.interface/Components.utils uses with Ci/Cu in suite. r=frg

OK, all the non-Calendar parts should now be covered. I'll uplift this to avoid future merge conflicts. Philipp needs to sort out the Calendar part when he deems fit. BTW, the directories are denoted as mail/ not /mail, but I removed the entire inflation of slashes ;-)

Comment on attachment 8958221 [details] [diff] [review] 1436605.patch - Seamonkey remainders [landed comment #34] Doing uplifts now and we don't want to get into a future merge mess.

Beta (TB 60) for the parts that haven't already landed on TB 60: https://hg.mozilla.org/releases/comm-beta/rev/ebb2e9fc3e3579b8854b50f87c249b8479ab7c15 https://hg.mozilla.org/releases/comm-beta/rev/653f2039952a976cdda8500d8b69a81cbefc3e98

I've filed bug 1458367 for Calendar. The patches here were mostly landed on TB 60, the ones that got landed on TB 61 were backported. So leaving the target milestone at TB 60.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/66d13a4d61ad follow-up: Replace left-over Components.classes/Components.interfaces. r=aceman

TB 60 beta 7 (BETA_60_CONTINUATION branch): https://hg.mozilla.org/releases/comm-beta/rev/1f026de51a4e11254805dd37f1eb2a495f27388c

Comment on attachment 8982869 [details] [diff] [review] 1436605-left-overs.patch Review of attachment 8982869 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. What about those in https://dxr.mozilla.org/comm-central/source/common/bindings/textbox.xml ?

They were fixed yesterday: https://hg.mozilla.org/comm-central/rev/d5a126bdcf6e73498a893b0ff2a1b6361e9ee2d7#l1.12