Closed
Bug 1509744
Opened 6 years ago
Closed 6 years ago
Port commands WebExtensions API
Categories
(Thunderbird :: Add-Ons: Extensions API, task)
Thunderbird
Add-Ons: Extensions API
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 66.0
People
(Reporter: Fallen, Assigned: Fallen)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
The commands API sounds simple to port, so I took a stab at it. There are only a few differences to the browser/ commands API, namely lack of pageAction and sidebarAction related code. This is expected though since we don't have those.
I have not ported the tests as they require mochitests, and those are not quite finished yet. We should add them with bug 1509246 or a followup. While I did mostly get mochitests running locally, there were a few utilities from BrowserTestUtils used in the browser mochitests that I didn't have time to replicate.
Assignee | ||
Comment 1•6 years ago
|
||
Assignee | ||
Comment 2•6 years ago
|
||
(and now with linter fixed)
Attachment #9027413 -
Attachment is obsolete: true
Attachment #9027413 -
Flags: review?(geoff)
Attachment #9027414 -
Flags: review?(geoff)
Comment 3•6 years ago
|
||
Comment on attachment 9027414 [details] [diff] [review]
Fix - v2
Review of attachment 9027414 [details] [diff] [review]:
-----------------------------------------------------------------
I'm giving this r+ without digging too deep because it's so similar to the Firefox version. I would like to see the tests even if we can't have them running yet. I can do a Try run.
My other concern is that we have other parts of primary UI such as the compose window that commands wouldn't run on. I'm not sure what this would look like, but my guess is a key in the manifest entry that specifies where commands should be active.
::: mail/components/extensions/jar.mn
@@ +8,5 @@
>
> content/messenger/parent/ext-addressBook.js (parent/ext-addressBook.js)
> content/messenger/parent/ext-browserAction.js (parent/ext-browserAction.js)
> content/messenger/parent/ext-composeAction.js (parent/ext-composeAction.js)
> + content/messenger/parent/ext-commands.js (parent/ext-commands.js)
In order, please! :-)
Attachment #9027414 -
Flags: review?(geoff) → review+
Assignee | ||
Comment 4•6 years ago
|
||
Order changed. I don't really have something viable for tests yet. I mainly copied the m-c tests from https://searchfox.org/comm-central/search?q=&path=browser_ext_commands but quickly ran into the issue that a lot of the tests were using CustomizableUI helpers in head.js that would need to be ported. I was aiming for a more explorative approach in porting those, and for that I'd like to be able to run the tests locally.
Ok to check in anyway, or do you want tests first?
Attachment #9027414 -
Attachment is obsolete: true
Flags: needinfo?(geoff)
Attachment #9029440 -
Flags: review+
Comment 5•6 years ago
|
||
I've been using attachment 9024581 [details] [diff] [review] on mozilla-central to run mochitests locally. I don't think they should stop this landing, but we should at least try to get them going first. I can also do a Try run with tests.
Flags: needinfo?(geoff)
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/714714d80d62
WebExtension Commands API. r=darktrojan
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment 7•6 years ago
|
||
I've landed this as I'm satisfied it passes the tests, (or would do if they were landed and mochitests worked). I had to add this line to ext-mail.json:
> "events": ["uninstall"],
Target Milestone: --- → Thunderbird 66.0
Updated•5 years ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•