The default bug view has changed. See this FAQ.

Patch to support Function Keys in WebExtensions

VERIFIED FIXED in Firefox 53

Status

()

Toolkit
WebExtensions: Frontend
VERIFIED FIXED
4 months ago
3 months ago

People

(Reporter: Laurent, Assigned: Laurent)

Tracking

({dev-doc-complete})

50 Branch
mozilla53
dev-doc-complete
Points:
---

Firefox Tracking Flags

(firefox53 verified)

Details

(Whiteboard: triaged)

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

4 months ago
Created attachment 8816746 [details] [diff] [review]
patch.txt

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20161025170400

Steps to reproduce:

In order to convert SDK addons to the new WebExtension technology, we
have to support Function Keys (F1, F2, etc. F12) on keyboards.

The attached patch adds the support of such keys in the "commands"
section of a WebExtension. This patch is quite simple, because the
code in browser/components/extensions/ext-commands.js does not even
needs to be changed. We only need to change the regular expression,
to allow these function keys.

Attached, there is also a test WebExtension (manifest.json/background.js)
to show the usage of several key combinations.

Please include this patch in Firefox sources, so it will allow us
to convert SDK addons to WebExtension.

Thank you.
(Assignee)

Comment 1

4 months ago
Created attachment 8816747 [details]
manifest.json

Manifest for the test WebExtension
(Assignee)

Comment 2

4 months ago
Created attachment 8816748 [details]
background.js

Background script for the WebExtension.
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit

Updated

4 months ago
Attachment #8816746 - Attachment is patch: true
Attachment #8816746 - Flags: review?(kmaglione+bmo)

Comment 3

3 months ago
Comment on attachment 8816746 [details] [diff] [review]
patch.txt

Review of attachment 8816746 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay. We've all been traveling for a company-wide all hands, which makes things a bit hectic.

This looks good. Thanks! Please add the checkin-needed keyword when you're ready for it to land.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=13eb84d41ca88653101a1205cac5dc5d330af21b
Attachment #8816746 - Flags: review?(kmaglione+bmo) → review+

Updated

3 months ago
Assignee: nobody → laurentconstantin
Status: UNCONFIRMED → NEW
Component: WebExtensions: General → WebExtensions: Frontend
Ever confirmed: true

Updated

3 months ago
Keywords: checkin-needed

Updated

3 months ago
Whiteboard: triaged

Comment 4

3 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/781d1e9b8586
Support Function Keys in WebExtensions. r=kmag
Keywords: checkin-needed

Comment 5

3 months ago
Updated https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/commands?document_saved=true
Keywords: dev-doc-complete

Comment 6

3 months ago
I had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=8132421&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/cd1610b7d0c474b53a63b6db8f434b9320eeed52
Flags: needinfo?(laurentconstantin)
(Assignee)

Comment 7

3 months ago
Hello,

A fixed patch is attached as patch2.txt.

My testing web extension was working, but not the automated test suite.

I guess I ran the automatic tests manually on the unpatched tree, so the tests were not really tested.

Now, I used on the patched tree :
  ./mach test browser/components/extensions/test/xpcshell/test_ext_manifest_commands.js
  ./mach test browser/components/extensions/test/browser/browser_ext_commands_onCommand.js
And this is apparently the good way to run tests. Correct me if this is wrong.

Sorry for my mistake.
Flags: needinfo?(laurentconstantin)
(Assignee)

Comment 8

3 months ago
Created attachment 8822906 [details] [diff] [review]
Fixed patch, with working test suite
(Assignee)

Updated

3 months ago
Attachment #8822906 - Attachment is obsolete: true
(Assignee)

Comment 9

3 months ago
Created attachment 8822908 [details] [diff] [review]
patch2.txt

The fixed patch, with working test case.

Comment 10

3 months ago
Thanks Laurent, if that patch is good to go, can you add "checkin-needed" to the keywords, then the sheriffs will try and autoland the patch for you.
Flags: needinfo?(laurentconstantin)
(Assignee)

Comment 11

3 months ago
I've added the "checkin-needed" keyword.
Flags: needinfo?(laurentconstantin)
Keywords: checkin-needed

Comment 12

3 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e12ce3afaf80
"Patch to support Function Keys in WebExtensions". r=kmag
Keywords: checkin-needed

Comment 13

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e12ce3afaf80
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 14

3 months ago
Thanks so much, Laurent! Your contribution has been added to our recognition wiki at https://wiki.mozilla.org/Add-ons/Contribute/Recognition#January_2017. 

Welcome onboard! We're happy to have you on the team.
I was able to reproduce the initial issue on Firefox 53.0a1 (2016-12-04) under Windows 10 64-bit.

Verified fixed on Firefox 53.0a1 (2017-01-09) under Windows 10 64-bit and Ubuntu 16.04 32-bit. The webextension is successfully installed and the following error is no longer thrown in Browser Console: http://pastebin.com/jRBJTLbk
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
You need to log in before you can comment on or make changes to this bug.