Ignore minimum_opera_version in WebExtensions manifest

VERIFIED FIXED in Firefox 58

Status

defect
P5
normal
VERIFIED FIXED
2 years ago
11 months ago

People

(Reporter: cr0ydon, Assigned: apoorvasingh2811, Mentored)

Tracking

({good-first-bug})

Trunk
mozilla58

Firefox Tracking Flags

(firefox58 verified)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170926190823

Steps to reproduce:

I want to support Firefox and Opera in my WebExtension, therefore I have added minimum_opera_version to the manifest file to prevent breakages in older Opera version I don't support.


Actual results:

Firefox is throwing an error, complaining about the minimum_opera_version key in the manifest file.

> addons.webextension.<unknown>	WARN	Loading extension 'null': Reading manifest: Error processing minimum_opera_version: An unexpected property was found in the WebExtension manifest.


Expected results:

Firefox should just ignore the minimum_opera_version key in the manifest file.
Component: Untriaged → WebExtensions: Compatibility
Product: Firefox → Toolkit
Mentor: bob.silverberg
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: good-first-bug
Priority: -- → P5
Assignee

Comment 1

2 years ago
Hi, I'm a beginner. Can I take this up?
Flags: needinfo?(bob.silverberg)
(In reply to apoorvasingh2811 from comment #1)
> Hi, I'm a beginner. Can I take this up?

Sure, thanks for offering to take this on.

To fix this bug you'll need to add an entry to the manifest schema, for an optional string property called "minimum_opera_version", which will be very much like the existing property for "minimum_chrome_version" at:

http://searchfox.org/mozilla-central/rev/298033405057ca7aa5099153797467eceeaa08b5/toolkit/components/extensions/schemas/manifest.json#17

You'll need to add a test to verify that this optional property is now ignored. It can be very much like the test we currently have at:

http://searchfox.org/mozilla-central/source/toolkit/components/extensions/test/xpcshell/test_ext_manifest_minimum_chrome_version.js

except that you'll need to specify the "minimum_opera_version" property in the test instead of the "minimum_chrome_version" property.

Finally, you'll need to add a line for your new test to the xpcshell.ini file at:

http://searchfox.org/mozilla-central/source/toolkit/components/extensions/test/xpcshell/xpcshell.ini

Please let me know if you have any questions.
Flags: needinfo?(bob.silverberg)
Comment on attachment 8914505 [details] [diff] [review]
Added the entry to the manifest schema, added the test, and made the required change in xpcshell.ini

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

Looks great, thanks for the patch! I'm giving it an r+ and will pass it onto a peer for a final review.
Attachment #8914505 - Flags: review?(mixedpuppy)
Attachment #8914505 - Flags: review?(bob.silverberg)
Attachment #8914505 - Flags: review+
Assignee

Comment 5

2 years ago
(In reply to Bob Silverberg [:bsilverberg] from comment #4)
> Comment on attachment 8914505 [details] [diff] [review]
> Added the entry to the manifest schema, added the test, and made the
> required change in xpcshell.ini
> 
> Review of attachment 8914505 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks great, thanks for the patch! I'm giving it an r+ and will pass it onto
> a peer for a final review.

Thank you so much for your guidance!
Since this is my contribution, I haven't been vouched by anyone yet. If you don't mind, can you please vouch for me!?
Assignee

Comment 6

2 years ago
(In reply to apoorvasingh2811 from comment #5)
> (In reply to Bob Silverberg [:bsilverberg] from comment #4)
> > Comment on attachment 8914505 [details] [diff] [review]
> > Added the entry to the manifest schema, added the test, and made the
> > required change in xpcshell.ini
> > 
> > Review of attachment 8914505 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Looks great, thanks for the patch! I'm giving it an r+ and will pass it onto
> > a peer for a final review.
> 
> Thank you so much for your guidance!
> Since this is my contribution, I haven't been vouched by anyone yet. If you
> don't mind, can you please vouch for me!?

my first contribution*
(In reply to apoorvasingh2811 from comment #5)
> 
> Thank you so much for your guidance!
> Since this is my contribution, I haven't been vouched by anyone yet. If you
> don't mind, can you please vouch for me!?

Certainly. Vouch for you where?
Assignee

Comment 8

2 years ago
(In reply to Bob Silverberg [:bsilverberg] from comment #7)
> (In reply to apoorvasingh2811 from comment #5)
> > 
> > Thank you so much for your guidance!
> > Since this is my contribution, I haven't been vouched by anyone yet. If you
> > don't mind, can you please vouch for me!?
> 
> Certainly. Vouch for you where?

Here : https://mozillians.org/en-US/u/apoorvasingh17/
Thanks!
(In reply to apoorvasingh2811 from comment #8)
> 
> Here : https://mozillians.org/en-US/u/apoorvasingh17/
> Thanks!

Done. Thanks for your contribution.
Attachment #8914505 - Flags: review?(mixedpuppy) → review+
Assignee: nobody → apoorvasingh2811

Comment 10

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9b0cdd852e9
Ignore minimum_opera_version in WebExtensions manifest. r=bsilverberg, r=mixedpuppy
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f9b0cdd852e9
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Comment 12

2 years ago
Verified as fixed in FF58.0b6 in Win 7 64 bit, MacOS 10.13.1
I tried loading the extension as a temporary add-on and installing it as a submitted add-on from the AMO site. Firefox is now ignoring minimum_opera_version from the manifest file.
Marking bug as verified fixed.
Status: RESOLVED → VERIFIED

Updated

11 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.