Closed Bug 1509744 Opened 6 years ago Closed 6 years ago

Port commands WebExtensions API

Categories

(Thunderbird :: Add-Ons: Extensions API, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 66.0

People

(Reporter: Fallen, Assigned: Fallen)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Fix - v1 (obsolete) (deleted) β€” β€” Splinter Review
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #9027413 - Flags: review?(geoff)
Attached patch Fix - v2 (obsolete) (deleted) β€” β€” Splinter Review
(and now with linter fixed)
Attachment #9027413 - Attachment is obsolete: true
Attachment #9027413 - Flags: review?(geoff)
Attachment #9027414 - Flags: review?(geoff)
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+
Attached patch Fix - v3 (deleted) β€” β€” Splinter Review
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+
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
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
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: