Embedded WebExtensions do not support options_ui manifest properties

VERIFIED FIXED in Firefox 51

Status

()

defect
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: noitidart, Assigned: kmag)

Tracking

({dev-doc-complete})

52 Branch
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51+ verified, firefox52- verified)

Details

Attachments

(4 attachments)

Reporter

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20160920155715

Steps to reproduce:

I created embedded webext. I set in manifest.json:

https://github.com/Noitidart/Profilist/blob/master/webextension/manifest.json#L7-L10

	"options_ui": {
		"page": "pages/options.html",
		"open_in_tab": true
	},


Actual results:

No button is shown for options page in about:addons please see attached screenshot.


Expected results:

Options button should show and it should open:
moz-extension://a55063ac-4f0b-4fe8-b3d2-95b8217fb8c8/pages/options.html
Reporter

Updated

3 years ago
Component: Untriaged → WebExtensions: Untriaged
Product: Firefox → Toolkit
Version: 50 Branch → 52 Branch
Reporter

Comment 1

3 years ago
For now I am using:

	"icons": {
		"16": "images/group.svg",
		"48": "images/group.svg",
		"64": "images/group.svg",
		"96": "images/group.svg",
		"128": "images/group.svg"
	},

I'm not sure what sizes to provide there, docs only really state what 48 is used for. Per the docs, I'm not sure if any of the other sizes have any application. I'm thinking 96 is used as 2x where HDPI is present.
Assignee

Comment 2

3 years ago
[Tracking Requested - why for this release]:

The transparent embedded WebExtension feature was introduced in Firefox 51, and developers should reasonably expect this feature to work with it.
Assignee: nobody → kmaglione+bmo
Status: UNCONFIRMED → ASSIGNED
Component: WebExtensions: Untriaged → Add-ons Manager
Ever confirmed: true
Summary: Embedded webext options_ui of manifest.json not respected → Embedded WebExtensions do not support options_ui manifest properties
Assignee

Comment 3

3 years ago
*was introduced in Firefox 51 by bug 1269342.
Depends on: 1269342
Comment hidden (mozreview-request)
Assignee

Updated

3 years ago
Attachment #8795908 - Flags: feedback?(lgreco)

Comment 5

3 years ago
mozreview-review
Comment on attachment 8795908 [details]
Bug 1306037: Support options_ui in embedded WebExtensions.  f?rpl

https://reviewboard.mozilla.org/r/81874/#review80474

::: testing/specialpowers/content/SpecialPowersObserverAPI.js:623
(Diff revision 1)
> +            // readManifest() will throw if we're loading an embedded
> +            // extension, so don't worry about locale errors in that
> +            // case.

Why does it throw on an embedded extension?  Can we catch just the specific error in that case so other errors don't get swallowed here?

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1172
(Diff revision 1)
> +
> +    if (addon.hasEmbeddedWebExtension) {
> +      let uri = NetUtil.newURI("webextension/manifest.json", null, aUri);
> +      let embeddedAddon = yield loadManifestFromWebManifest(uri);
> +      if (embeddedAddon.optionsURL) {
> +        addon.optionsURL = embeddedAddon.optionsURL;

Maybe log a warning or even generate an error here if both halves of the addon have options urls?
Attachment #8795908 - Flags: review?(aswan) → review+
Assignee

Comment 6

3 years ago
mozreview-review-reply
Comment on attachment 8795908 [details]
Bug 1306037: Support options_ui in embedded WebExtensions.  f?rpl

https://reviewboard.mozilla.org/r/81874/#review80474

> Why does it throw on an embedded extension?  Can we catch just the specific error in that case so other errors don't get swallowed here?

Because the path to the manifest is wrong. This code is redundant in extensions loaded by the add-on manager, and if `readManifest` throws here, we'll get the same error when we try to load the same extension, so we're not losing any important errors here.

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bc3137d7d869
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee

Comment 9

3 years ago
Comment on attachment 8795908 [details]
Bug 1306037: Support options_ui in embedded WebExtensions.  f?rpl

Approval Request Comment
[Feature/regressing bug #]: Bug 1269342
[User impact if declined]: Extensions which attempt to provide options to users via the options_ui feature of embedded WebExtensions will fail to do so in versions prior to 52.
[Describe test coverage new/current, TreeHerder]: The related features are well-covered by existing tests, and new tests were added for this change.
[Risks and why]: Low. The changes required to implement this are fairly simple, and should only affect embedded WebExtensions. The changes outside of XPIProvider.jsm are all test-only.
[String/UUID change made/needed]: None.
Attachment #8795908 - Flags: feedback?(lgreco) → approval-mozilla-aurora?
Hi Florin, can we have some QA's help to verify this?
Flags: qe-verify+
Flags: needinfo?(florin.mezei)

Comment 11

3 years ago
Verified this on Windows 7 - x64, using the "Install from File" installation method from about:addons and obtained the following results:

1. Nightly (Version 52.0a1, Build ID 20160928065829)
Embedded web-extension is installed but no "Options" button listed in about:addons -> http://screencast.com/t/wc1DEjgtS2g
Browser console errors: 
- https://pastebin.mozilla.org/8915296
- https://pastebin.mozilla.org/8915297

2. Nightly (Version 52.0a1, Build ID 20161002030227)
Embedded web-extension cannot be installed; corrupt message displayed;
Browser console errors at install: 
- https://pastebin.mozilla.org/8915293 
- https://pastebin.mozilla.org/8915294

Attaching used embedded web-extension. (is the one mentioned at comment #0, but with other versions range)

Kris Maglione [:kmag] : Is the "locales" error from the browser console a result of this fix or I missed something out?
Flags: needinfo?(florin.mezei) → needinfo?(kmaglione+bmo)

Comment 12

3 years ago
Posted file Profilist-t.xpi
Hi Valentina,
the error related to the locales is due to a different issue: Bug 1206858 " Localizations for packed embedded webextensions searched for in extension root". 

Bug 1206858 has already a patch attached, but the fix is not landed yet, to be able to test the fix related to this issue without waiting for that fix, you can workaround the issue by:

- unpacking the above xpi
- copy the _locales dir into the unpacked add-on root dir
- repack the modified addon into a new xpi file
I tried the attached addon with the above workaround applied, but unfortunately there are still a scenario where preferences from the embedded webextension can still fail to be shown in the "about:addons" page:

The reason is that, with the fix applied by the patch attached to this issue, the addon preferences can be successfully rendered only once the embedded webextension has been started, and the attached addon currently doesn't call the `startup()` method if the promise returned by `installNativeMessaging()` is rejected (and it works if the bootstrap.js file is changed so that the embedded webextension is started even if the above promise is rejected).

(If the embedded webextension is not started, the preferences are not shown because during the rendering of the "about:addons" page, the `getURLForExtension` method raises a "Called getURLForExtension on unmapped extension Profilist@jetpack  ExtensionManagement.jsm:121" exception, because the addon uuid is allocated and stored for a webextension only when the it is actually started).

Comment 15

3 years ago
Hi Luca and thanks for your quick reply!

So, we should wait for the fix from https://bugzilla.mozilla.org/show_bug.cgi?id=1206858 to land? will that fix catch the other scenario you found, that fails?
Reporter

Comment 16

3 years ago
(In reply to Luca Greco [:rpl] from comment #14)
> I tried the attached addon with the above workaround applied, but
> unfortunately there are still a scenario where preferences from the embedded
> webextension can still fail to be shown in the "about:addons" page:
> 
> The reason is that, with the fix applied by the patch attached to this
> issue, the addon preferences can be successfully rendered only once the
> embedded webextension has been started, and the attached addon currently
> doesn't call the `startup()` method if the promise returned by
> `installNativeMessaging()` is rejected (and it works if the bootstrap.js
> file is changed so that the embedded webextension is started even if the
> above promise is rejected).
> 
> (If the embedded webextension is not started, the preferences are not shown
> because during the rendering of the "about:addons" page, the
> `getURLForExtension` method raises a "Called getURLForExtension on unmapped
> extension Profilist@jetpack  ExtensionManagement.jsm:121" exception, because
> the addon uuid is allocated and stored for a webextension only when the it
> is actually started).

Thanks Luca for testing the attachment so deeply! I think it makes sense that if the webext is not started then the options button does not show. Clicking the options button would open a webext page. So if the webext didn't start, then showing the options button would open an "error" page.

So it should probably be on me to handle it when native messaging fails to install. The addon completely relies on that (options affect across all profiles, so are stored in profiles.ini). If it fails to install, I'll have to give the failure reason to the user. The user can then post an issue on my bug tracker and I can troubleshoot with them.
Assignee

Updated

3 years ago
Flags: needinfo?(kmaglione+bmo)
Comment on attachment 8795908 [details]
Bug 1306037: Support options_ui in embedded WebExtensions.  f?rpl

Fix a Web Extension related issue which will be in 51. Take it in 51 aurora.
Attachment #8795908 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Track 51+ as this feature will be in 51
(In reply to ValentinaP from comment #15)
> Hi Luca and thanks for your quick reply!
> 
> So, we should wait for the fix from
> https://bugzilla.mozilla.org/show_bug.cgi?id=1206858 to land? will that fix
> catch the other scenario you found, that fails?

My apologies, the bug linked in my previous comment contains a typo, the right bugzilla issue is Bug 1306858.

Bug 1306858 has now been landed and uplifted to 51 but, as I described in Comment 14 (and noitidart confirmed by Comment 16), the version of the addon currently attached to this issue doesn't start the embedded webextension if the promise returned by installNativeMessaging is rejected, and in this scenario the preferences button is not rendered.

I'm going to attach an xpi with a simple test addon.
xpi file of a simple test addon with a options_ui page defined in the embedded webextension manifest.

Comment 22

3 years ago
(In reply to Luca Greco [:rpl] from comment #21)
> Created attachment 8798033 [details]
> test-options-ui-in-embedded-webextension.xpi
> 
> xpi file of a simple test addon with a options_ui page defined in the
> embedded webextension manifest.

Thanks a lot, Luca!
Using your add-on I've tested on:
1. Nightly (v. 52.0a1 - Build ID 20161005030211) 
2. Aurora (v. 51.0a2 - Build ID 20161005004013)
For both builds the "Options" button are displayed, but only after page refresh: http://screencast.com/t/ScUGxxChzXrz

Should it be listed directly after install, or it's ok like this?
(In reply to ValentinaP from comment #22)
> (In reply to Luca Greco [:rpl] from comment #21)
> > Created attachment 8798033 [details]
> > test-options-ui-in-embedded-webextension.xpi
> > 
> > xpi file of a simple test addon with a options_ui page defined in the
> > embedded webextension manifest.
> 
> Thanks a lot, Luca!
> Using your add-on I've tested on:
> 1. Nightly (v. 52.0a1 - Build ID 20161005030211) 
> 2. Aurora (v. 51.0a2 - Build ID 20161005004013)
> For both builds the "Options" button are displayed, but only after page
> refresh: http://screencast.com/t/ScUGxxChzXrz
> 
> Should it be listed directly after install, or it's ok like this?

the above screencast seems to present the expected behavior when this fix is applied.

Unfortunately if the about:addons page is already loaded in a tab when the addon is installed, the preferences button will not be shown until the about:addons page has been reloaded, for a reason similar to the one described in comment 14 (the about:addons page is opened, it is refreshed once the addon has been installed, but the embedded webextension has not been started yet)
Tracking - for 52 and I think this fix is covered.
This change is not yet covered in the MDN docs related to the "Embedded WebExtensions" feature (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Embedded_WebExtensions), in particular the following details have to be mentioned/changed in the updated docs:

- the "option_ui" from the Embedded WebExtension is now supported (e.g. where the current page says "For example, the add-on's preferences UI will still use the old system", it should become more like "can still use the old system", and, even if it is unrelated, the link in the MDN page related to the above text paragraph currently points to a non-existent section of the same page)

- when specified, the "option_ui" from the Embedded WebExtension manifest will override the Addon Preference UI (at install time, and even if the legacy hybrid addon has its own preferences UI configured in the install.rdf)

- the "option_ui" from the Embedded WebExtension will work only (and in the "about:addons" page the Preferences button related to the addon will be visible) if the embedded webextension has been started (in other words, the "embeddedWebExtension.startup()" method should have been called at least once)

as known issues of the current implementation of the "option_ui" from the Embedded WebExtension:

- if the embedded webextension is not started, in the "about:addons" page, the Preferences button related to the hybrid addon will never be visible (an exception logged is logged in the browser console)

- if the "about:addons" page is already opened in a tab when the hybrid addon with the option_ui in the embedded webextension is installed, the Preferences button will not be visible until the next reload of the "about:addons" page (and the same exception message from the above known issue is logged in the browser console)
Keywords: dev-doc-needed

Updated

3 years ago
Blocks: 1310604
Based on comment #22, #23 and #25, there doesn't seem to be any additional verification needed here, so marking this as Verified.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
I've added(In reply to Luca Greco [:rpl] from comment #25)
> This change is not yet covered in the MDN docs related to the "Embedded
> WebExtensions" feature

I removed the note you referred to, since it seems to be no longer true. I also added:

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Embedded_WebExtensions#Preferences

...which I hope covers the current situation. Please let me know if it looks OK.
Flags: needinfo?(lgreco)
Thanks Will, the updated Preferences section looks great!

There is something that I'd like to mention in the above section if possible (I mean if we can find a formula that doesn't make it more confusing):

if the embedded webextension defines an option_ui page, the `startup` method should be called synchronously during the initial addon load (I mean "without waiting something asynchronous to happen"), so that the Preferences button can immediatelly appear (and work correctly), like it does for a regular webextension.

what do you think?
Flags: needinfo?(lgreco)
OK, I've tried to clarify this here: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Embedded_WebExtensions.

Marking as dev-doc-complete, but do please let me know if I need anything else.
It looks great, thanks Will!
You need to log in before you can comment on or make changes to this bug.