Closed
Bug 1497705
Opened 2 years ago
Closed 2 years ago
Errors in common/, hidden by ESLint config file
Categories
(Thunderbird :: Add-Ons: Extensions API, task)
Thunderbird
Add-Ons: Extensions API
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 64.0
People
(Reporter: darktrojan, Assigned: darktrojan)
Details
Attachments
(1 file, 1 obsolete file)
11.48 KB,
patch
|
jorgk-bmo
:
review+
|
Details | Diff | Splinter Review |
I've just uncovered a bug in ChromeManifest.jsm that was hidden by the "no-undef" rule being turned off in common/.eslintrc.js. That file is unnecessary, so I removed it and uncovered another bug. Not really that happy about that.
Assignee | ||
Comment 1•2 years ago
|
||
Comment 2•2 years ago
|
||
Comment on attachment 9015728 [details] [diff] [review] 1497705-eslint-common-1.diff Review of attachment 9015728 [details] [diff] [review]: ----------------------------------------------------------------- Does a quick review make you happier? ::: common/src/ChromeManifest.jsm @@ +94,5 @@ > * @param {String[]} flags An array of raw flag values in the form key=value. > * @return {Boolean} True, if the flags match the options provided in the constructor > */ > _parseFlags(flags) { > + if (!flags.length) { Do you prefer this or flags.length == 0?
Attachment #9015728 -
Flags: review?(jorgk) → review+
Assignee | ||
Comment 3•2 years ago
|
||
Actually I do prefer == 0, but it's all the same :) Ship it! This really should go on beta too, I don't know if it applies cleanly there.
Keywords: checkin-needed
Comment 4•2 years ago
|
||
You can ship it if there's a merge in ~5 hours.
Comment 5•2 years ago
|
||
Comment on attachment 9015728 [details] [diff] [review] 1497705-eslint-common-1.diff Review of attachment 9015728 [details] [diff] [review]: ----------------------------------------------------------------- ::: common/src/extensionSupport.jsm @@ +26,5 @@ > function extensionDefaults() { > // Fetch enabled non-bootstrapped add-ons. > let enabledAddons = Services.dirsvc.get("XREExtDL", Ci.nsISimpleEnumerator); > for (let addonFile of fixIterator(enabledAddons, Ci.nsIFile)) { > + ExtensionSupport.loadAddonPrefs(addonFile); Does that mean it didn't work?
Assignee | ||
Comment 6•2 years ago
|
||
It didn't work, but it doesn't throw an exception, because that code never runs, because enabledAddons is always empty. Fortunately we call ExtensionSupport.loadAddonPrefs from the legacy extension handler anyway, so even if it did work, it's redundant. I'm going to remove the whole thing.
Assignee | ||
Comment 7•2 years ago
|
||
I could go step further and fix the name of extensionSupport.jsm, but I'll save that for another day. ;-)
Attachment #9015728 -
Attachment is obsolete: true
Attachment #9015778 -
Flags: review?(jorgk)
Assignee | ||
Updated•2 years ago
|
Keywords: checkin-needed
Comment 8•2 years ago
|
||
Comment on attachment 9015778 [details] [diff] [review] 1497705-eslint-common-2.diff (In reply to Geoff Lankow (:darktrojan) from comment #6) > It didn't work, but it doesn't throw an exception, because that code never > runs, because enabledAddons is always empty. Fortunately we call > ExtensionSupport.loadAddonPrefs from the legacy extension handler anyway, so > even if it did work, it's redundant. Can you enlighten me here a little bit. This was implemented in the mozilla59/60 days when M-C scrapped the code that read the defaults. It's still actively working here: https://dxr.mozilla.org/comm-esr60/source/common/src/extensionSupport.jsm# So when did Services.dirsvc.get("XREExtDL", Ci.nsISimpleEnumerator); start returning nothing? Most likely when the non-bootstrapped add-ons disappeared, right, during mozilla 61?
Attachment #9015778 -
Flags: review?(jorgk) → review+
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/fe10a3a8d48e Remove ESLint rule overrides in common/ and broken code they were hiding. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Updated•2 years ago
|
Target Milestone: --- → Thunderbird 64.0
Updated•1 year ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•