1404746 - Ignore minimum_opera_version in WebExtensions manifest

Status: VERIFIED FIXED

(WebExtensions :: Compatibility, defect, P5)

Description

[Croydon]

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.

Comment 1

[apoorvasingh2811]

Hi, I'm a beginner. Can I take this up?

Comment 2

[Bob Silverberg [:bsilverberg]]

(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.

Comment 3

[apoorvasingh2811]

Attachment: Added the entry to the manifest schema, added the test, and made the required change in xpcshell.ini

Comment 4

[Bob Silverberg [:bsilverberg]]

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.

Comment 5

[apoorvasingh2811]

(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!?

Comment 6

[apoorvasingh2811]

(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*

Comment 7

[Bob Silverberg [:bsilverberg]]

(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?

Comment 8

[apoorvasingh2811]

(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!

Comment 9

[Bob Silverberg [:bsilverberg]]

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

Done. Thanks for your contribution.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9b0cdd852e9
Ignore minimum_opera_version in WebExtensions manifest. r=bsilverberg, r=mixedpuppy

Comment 11

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f9b0cdd852e9

Comment 12

[Madalin Cotetiu]

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.