Closed Bug 1387813 Opened 7 years ago Closed 7 years ago

Port Bug 1375125 - Remove nsILocalFile to C-C

Categories

(Thunderbird :: General, enhancement)

Product:
Thunderbird
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 57.0

People

(Reporter: jorgk-bmo, Assigned: Paenglab)

Details

(Keywords: addon-compat)

Attachments

(2 files, 2 obsolete files)

bug1387813.patch
84.29 KB, patch
jorgk-bmo
: review+
Details | Diff | Splinter Review
bug1387813-suite.patch
31.85 KB, patch
jorgk-bmo
: review+
Details | Diff | Splinter Review 
We have Components.interfaces.nsILocalFile 1000 thousand times in JS.

Suite also has:
#include "nsILocalFileMac.h"

Looks like the JS part is mainly a sed job:
-const nsILocalFile = CC("@mozilla.org/file/local;1", "nsILocalFile",
+const nsIFile = CC("@mozilla.org/file/local;1", "nsIFile",

M-C landed:
https://hg.mozilla.org/mozilla-central/rev/f904bc721e7d
and a fix to this in
https://hg.mozilla.org/mozilla-central/rev/56b1b043078b
so we've been warned that a global replace isn't easy.
Oh, looks like nsILocalFileMac hasn't been removed. Mailnews also uses it.
Attached patch bug1387813.patch (obsolete) — — Splinter Review 
This patch replaces all nsILocalFile with nsIFile. nsILocalFileMac and nsILocalFileWin are not touched. It has some whitespace changes in it as my editor automatically removes them at the end of lines.
Attachment #8894202 - Flags: review?(jorgk)
Uh, somehow I didn't got the calendar files. Looking at it.
You missed

.\suite\modules\WindowsJumpLists.jsm:    var file = Services.dirsvc.get("XREExeF", Ci.nsILocalFile);

and all these:

.\calendar\base\content\calendar-dnd-listener.js:                                        .createInstance(Components.interfaces.nsILocalFile);
.\calendar\base\content\import-export.js:        const nsILocalFile = Components.interfaces.nsILocalFile;
.\calendar\base\content\import-export.js:                                          .createInstance(nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:                                    .createInstance(Components.interfaces.nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:        var profileDir = this.dirService.get("ProfD", Components.interfaces.nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:        var profileDir = this.dirService.get("ProfD", Components.interfaces.nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:                   .createInstance(Components.interfaces.nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:        var evoDir = this.dirService.get("Home", Components.interfaces.nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:                                          Components.interfaces.nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:        let profileRoot = this.dirService.get("DefProfRt", Components.interfaces.nsILocalFile);
.\calendar\base\content\dialogs\calendar-migration-dialog.js:        var profileRoot = this.dirService.get("DefProfRt", Components.interfaces.nsILocalFile);
.\calendar\base\content\preferences\alarms.js:     * Converts the given file url to a nsILocalFile
.\calendar\base\content\preferences\alarms.js:     * @return            The corresponding nsILocalFile.
.\calendar\base\content\preferences\alarms.js:        // Convert the file url into a nsILocalFile
.\calendar\base\src\calCalendarManager.js:        let storageSdb = Services.dirsvc.get("ProfD", Components.interfaces.nsILocalFile);
.\calendar\base\src\calTimezoneService.js:                                 .createInstance(Components.interfaces.nsILocalFile);
.\calendar\base\src\calTimezoneService.js:                                 .createInstance(Components.interfaces.nsILocalFile);
.\calendar\base\src\calUtils.js:        let dir = Services.dirsvc.get("ProfD", Components.interfaces.nsILocalFile);
.\calendar\lightning\content\lightning-item-iframe.js:            let file = files.getNext().QueryInterface(Components.interfaces.nsILocalFile);
.\calendar\lightning\content\lightning-item-iframe.js:        lastDirectory.mValue = file.parent.QueryInterface(Components.interfaces.nsILocalFile);
.\calendar\providers\ics\calICSCalendar.js:                    // nsILocalFile.  That's not the end of the world.  We can
.\calendar\providers\storage\calStorageCalendar.js:            let storageSdb = Services.dirsvc.get("ProfD", Components.interfaces.nsILocalFile);
.\calendar\providers\wcap\calWcapUtils.js:                                        .createInstance(Components.interfaces.nsILocalFile);
.\chat\components\src\imAccounts.js:      let lastCrash = Services.dirsvc.get("UAppData", Ci.nsILocalFile);
.\editor\ui\composer\content\ComposerCommands.js:        tempLocalFile = fileHandler.getFileFromURLSpec(urlstring).QueryInterface(Components.interfaces.nsILocalFile);
.\editor\ui\composer\content\ComposerCommands.js:         // If a local file, we must create a new uri from nsILocalFile
.\editor\ui\composer\content\editorUtilities.js:                                                    Components.interfaces.nsILocalFile);
.\editor\ui\composer\content\editorUtilities.js:        fileDir = filePicker.file.parent.QueryInterface(Components.interfaces.nsILocalFile);
.\editor\ui\composer\content\editorUtilities.js:                                       Components.interfaces.nsILocalFile, fileDir);
.\im\components\profileMigrator.js:        prof = root.clone().QueryInterface(Ci.nsILocalFile);
.\im\components\profileMigrator.js:        prof = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsILocalFile)
.\im\content\blist.js:                             .createInstance(Ci.nsILocalFile);
.\im\content\preferences\privacy.js:    var logFolder = Services.dirsvc.get("ProfD", Ci.nsILocalFile);
.\im\content\preferences\privacy.js:      let parent = logFolder.parent.QueryInterface(Ci.nsILocalFile);
.\suite\browser\navigator.js:                              Components.interfaces.nsILocalFile);
.\suite\browser\navigator.js:                                   Components.interfaces.nsILocalFile,
.\suite\browser\nsBrowserContentHandler.js:      fl.get("ProfD", Components.interfaces.nsILocalFile);
.\suite\browser\pageinfo\pageInfo.js:  const nsILocalFile = Components.interfaces.nsILocalFile;
.\suite\browser\pageinfo\pageInfo.js:    return fp.file.QueryInterface(nsILocalFile);
.\suite\common\utilityOverlay.js:               Components.interfaces.nsILocalFile);
.\suite\common\utilityOverlay.js:  return Services.dirsvc.get("Desk", Components.interfaces.nsILocalFile);
.\suite\common\utilityOverlay.js:         uri.file.QueryInterface(Components.interfaces.nsILocalFile) : null;
.\suite\common\bookmarks\bookmarksManager.js:    var backupsDir = dirSvc.get("Desk", Components.interfaces.nsILocalFile);
.\suite\common\downloads\downloadmanager.js:                                           Components.interfaces.nsILocalFile,
.\suite\common\downloads\downloadmanager.js:    var parent = file.parent.QueryInterface(Components.interfaces.nsILocalFile);
.\suite\common\pref\pref-applications.js: * - nsILocalFile, stores the current client-side feed reading app if one has
.\suite\common\pref\pref-applications.js:                                   Components.interfaces.nsILocalFile);
.\suite\common\src\nsSessionStartup.js:                                          Components.interfaces.nsILocalFile);
.\suite\common\src\nsSessionStore.js:    this._sessionFile = Services.dirsvc.get("ProfD", Components.interfaces.nsILocalFile);
.\suite\common\src\nsSuiteGlue.js:        Services.dirsvc.get("ProfD", Components.interfaces.nsILocalFile);
.\suite\common\src\nsSuiteGlue.js:    var profileDir = Services.dirsvc.get("ProfD", Components.interfaces.nsILocalFile);
.\suite\common\src\nsSuiteGlue.js:        var bookmarksHTMLFile = Services.dirsvc.get("BMarks", Components.interfaces.nsILocalFile);
.\suite\common\src\nsSuiteGlue.js:        var bookmarksFile = Services.dirsvc.get("BMarks", Components.interfaces.nsILocalFile);
.\suite\common\src\nsSuiteGlue.js:        BookmarkHTMLUtils.exportToFile(Services.dirsvc.get("BMarks", Components.interfaces.nsILocalFile)).then(
.\suite\common\src\nsSuiteGlue.js:                                     Components.interfaces.nsILocalFile,
.\suite\common\src\nsSuiteGlue.js:                                                                    Components.interfaces.nsILocalFile));
.\suite\common\src\nsSuiteGlue.js:      // nsILocalFile, but it does fallback gracefully.
.\suite\common\tests\browser\browser_346337.js:               .get("TmpD", Components.interfaces.nsILocalFile);
.\suite\common\tests\browser\browser_346337.js:             .get("TmpD", Components.interfaces.nsILocalFile);
.\suite\common\tests\browser\browser_466937.js:             .get("TmpD", Components.interfaces.nsILocalFile);
.\suite\feeds\src\FeedConverter.js:                                                     Components.interfaces.nsILocalFile);
.\suite\feeds\src\FeedWriter.js:          var file = Services.dirsvc.get("XREExeF", Components.interfaces.nsILocalFile);
.\suite\feeds\src\FeedWriter.js:                                           Components.interfaces.nsILocalFile);
.\suite\feeds\src\FeedWriter.js:                                                         Components.interfaces.nsILocalFile);
.\suite\feeds\src\FeedWriter.js:          Services.prefs.setComplexValue(getPrefAppForType(feedType), Components.interfaces.nsILocalFile,
.\suite\feeds\src\FeedWriter.js:          Services.prefs.setComplexValue(getPrefAppForType(feedType), Components.interfaces.nsILocalFile,
.\suite\mailnews\messengerdnd.js:                      .QueryInterface(Components.interfaces.nsILocalFile);
.\suite\mailnews\msgHdrViewOverlay.js:      var destDirectory = dirPrimitive.value.QueryInterface(Components.interfaces.nsILocalFile);
.\suite\mailnews\compose\MsgComposeCommands.js:// attachedLocalFile must be a nsILocalFile
.\suite\mailnews\compose\MsgComposeCommands.js:    var parent = file.parent.QueryInterface(Components.interfaces.nsILocalFile);
.\suite\mailnews\compose\MsgComposeCommands.js:                                   Components.interfaces.nsILocalFile, parent);
.\suite\mailnews\compose\MsgComposeCommands.js:    var currentFile = attachments.getNext().QueryInterface(Components.interfaces.nsILocalFile);
Attached patch bug1387813.patch (obsolete) — — Splinter Review 
This should be all now.
Attachment #8894202 - Attachment is obsolete: true
Attachment #8894202 - Flags: review?(jorgk)
Attachment #8894203 - Flags: review?(jorgk)
Oops, didn't catch suite.
Right, and also:
.\im\components\profileMigrator.js:        prof = root.clone().QueryInterface(Ci.nsILocalFile);screen__theme
.\im\components\profileMigrator.js:        prof = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsILocalFile)
.\im\content\blist.js:                             .createInstance(Ci.nsILocalFile);
Attached patch bug1387813.patch — — Splinter Review 
With the missing IM files.

I'll put the suite part in a separate patch.
Attachment #8894203 - Attachment is obsolete: true
Attachment #8894203 - Flags: review?(jorgk)
Attachment #8894205 - Flags: review?(jorgk)
Suite only patch. Again, no chsnges to nsILocalFileMac or nsILocalFileWin.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8894206 - Flags: review?(frgrahl)
This should be: Again, no changes to nsILocalFileMac or nsILocalFileWin.
Comment on attachment 8894206 [details] [diff] [review]
bug1387813-suite.patch

Looks good.
Attachment #8894206 - Flags: review?(frgrahl) → review+
> Looks good.

I concur. Many thanks.
Comment on attachment 8894205 [details] [diff] [review]
bug1387813.patch

Also looking good. Let me land this in a minute with an improved commit message.
Attachment #8894205 - Flags: review?(jorgk) → review+
Sorry, this has still not landed since I'm still staring at it, especially to see whether we did any mistakes like the ones corrected in https://hg.mozilla.org/mozilla-central/rev/56b1b043078b. We haven't.

However, I find this:
  let nsLocalFile = Components.Constructor("@mozilla.org/file/local;1",
                                           "nsIFile", "initWithPath");
rather unfortunate, it's used in function openLocalDirectory() in removeAccount.js and in function openProfileDirectory() in aboutSupport.js. The variable name nsLocalFile is really badly chosen now, but M-C maintained the bad naming here:
https://hg.mozilla.org/mozilla-central/rev/f904bc721e7d#l160.11

Since our aboutSupport.js is aligned with theirs, it's best to go with the flow.

Staring at the patch I also found some errors:
-  if (aFile instanceof Ci.nsILocalFileWin) {
+  if (aFile instanceof Ci.nsIFileWin) {

-  if (aFile instanceof Ci.nsILocalFileMac) {
+  if (aFile instanceof Ci.nsIFileMac) {

In fact, the hunks in im/content/preferences/applications.js are wrong or only white-space changes, so I'll remove them.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/46e8d4d1250d
Port bug 1375125: Replace nsILocalFile with nsIFile in C-C. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Thank to fix the tricky renames.
OK, I landed both patches together without the hunk in im/content/preferences/applications.js.

Fingers crossed that we didn't miss anything.
Target Milestone: --- → Thunderbird 57.0
Keywords: addon-compat
OS: Unspecified → All
Hardware: Unspecified → All
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: