1243467 - JPM xpi - Error when addon options should appear

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Description

[gardan]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151229124331

Steps to reproduce:

1) Create basic addon (jpm init is enough)
2) Add preferences to package.json (copy paste from mozilla documentation - or use your own):

"preferences": [{
        "name": "somePreference",
        "title": "Some preference title",
        "description": "Some short description for the preference",
        "type": "string",
        "value": "this is the default string value"
    },
    {
        "description": "How many of them we have.",
        "name": "myInteger",
        "type": "integer",
        "value": 8,
        "title": "How Many?"
    }]

3) Create xpi using "jpm xpi" and install it on mobile device (tested on android firefox beta and nightly) - or use jpm-mobile
4) Go to menu -> tools -> extensions and click on your addon to display options defined in package.json



Actual results:

Options do not appear.

Using remote debugging you should be able to see this error:

TypeError: doc.getElementById(...) is null
Stack trace:
onAddonOptionsDisplayed@resource://gre/modules/commonjs/toolkit/loader.
js -> resource://gre/modules/commonjs/sdk/preferences/native-
options.js:37:20
Observer<.observe@resource://gre/modules/commonjs/toolkit/loader.js ->
resource://gre/modules/commonjs/sdk/system/events.js:81:7
showDetails/xhr.onload@chrome://browser/content/aboutAddons.js:378:11



Expected results:

Addon options should appear. It works ok with firefox PC version.

It also works on all platforms when creating xpi using cfx (just with a little changes according to differences between cfx and jpm)

Comment 1

[gardan]

Kevin, is there any workaround for this? According to the article about cfx, only jpm is supported and addons built with cfx are no longer accepted for add-on submission. If there is no way how to make it work, then it should be critical bug.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

I have no idea. The addon sdk team would need to respond to this bug. Poking an Addon SDK team member.

Comment 3

[gardan]

I made some research and find a way how to make it work under current versions but Android Nightly. Maybe it can help someone with the same problem..

Tested versions:
Firefox desktop - ver.: 44.0.2 (win) 44.0.1 (lin) - works
Firefox desktop nightly - ver.: 47.0a1 (lin 64) - works
Firefox for Android - ver.: 44.0.2 - works
Firefox for Android beta - ver.: 45.0b4 - works
Firefox for Android nightly - ver.: 47.0a1 - not working (actually I didnt find ANY addon which works with latest Android nightly)


I had to do few changes inside xpi made by jpm:

1) add chrome.manifest with content: content addon123-options options/

2) create options folder

3) create options.xul file inside options folder (the content was generate from package.json and it's inspired by cfx options.xul):
<?xml version="1.0" ?>
<vbox xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
  <setting data-jetpack-id="your-jetpack-id@jetpack" pref="[e-mail]" pref-name="somePreference" title="Some preference title" type="string">
    Some short description for the preference
  </setting>
  <setting data-jetpack-id="your-jetpack-id@jetpack" pref="[e-mail]" pref-name="myInteger" title="How Many?" type="integer">
    How many of them we have.
  </setting>
</vbox>

4) change optionsUrl inside install.rdf and replace its value with: chrome://addon123-options/content/options.xul (original value was wierd: data:text/xml,&lt;placeholder/&gt; )

5) create prefs.js for default preferences (defaults/preferences/prefs.js - again generated from package.json):
pref("[e-mail]", "this is the default string value");
pref("[e-mail]", "8");

6) remove all preferences from pacakge.json (if it stays there, then you would see options on desktop firefox twice)

--------------

All suggestions/comments are welcome... I really want this to be fixed.

Comment 4

[Matteo Ferretti [:zer0] [:matteo]]

I'm not working actively on SDK anymore, kumar is in charge of jpm nowaday.

Kumar, could you have a look on this? Thanks!

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

This looks like something that have to be looked from an addon-sdk point of view (to be more specific: how the differences between an xpi built from the previous cfx tool and the current jpm tool presents a different runtime behavior over the different releases of Firefox for Android),
so I think that it would be better if I look into it first.

Changing the needinfo to me.

Comment 6

[Dave Royal]

See bug 1167246 
Since Fx44, when AMO rejects cfx addons, simple prefs has been broken on fennec.
The partial workround (set options with about:config) is user-unfriendly.
I think it should be a critical bug.

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

I took a look at this, and generating an "options.xul" to fix the issue on Firefox for Android, as suggested in the previous comments, has the unfortunate side-effect of rendering the preferences twice where the native-options are working correctly (e.g. Firefox on desktop systems).

Because of the above reason, I'm not sure that restoring the cfx behaviour in the jpm tool is the best approach to fix this issue.

Nevertheless once the issue is fixed in the "native-options" addon-sdk module, we still have Firefox for Android versions which needs the fix (e.g. from the >= 44 to the < 48), and so I tried a different approach to immediately workaround the issue in a more user-friendly and addon-developer-friendly way, by creating a small npm package which can be used as a dependency with jpm:

- small example addon: https://gist.github.com/rpl/7e009f3a504128605ae2feef13c95a29
- npm package: https://github.com/rpl/fennec-addon-preferences-workaround

To test the small example addon:

> git clone https://gist.github.com/rpl/7e009f3a504128605ae2feef13c95a29 test-fennec-addon
> cd  test-fennec-addon
>  
> # download the 'fennec-addon-preferences-workaround' package
> npm install
>  
> # run it on the Firefox Desktop
> jpm run
>  
> # package it as an xpi and then install and run it on Firefox for Android
> jpm xpi

Comment 8

[Dave Royal]

(In reply to Luca Greco [:rpl] from comment #7)
Thank you.

I used the workaround on my addon https://addons.mozilla.org/en-US/firefox/addon/mudcat-browser-tools/
The prefs now appear in fennec except for the control button:
    "name": "readmeButton",   
    "label": "ReadMe",    
    "title": "ReadMe",        
    "description": "What it does, preferences, caveats", 
    "type": "control"
The 'Readme' isn't visible and tapping it does nothing. 

The new version is here:
https://dl.dropboxusercontent.com/u/862480/temp/mudcat_browse_tools%40davero.addons.mozilla.org-1.2.1.xpi

Can that be fixed? (It's not very important in my case!)

Also, what's the significance of Fx 48? Is the problem fixed in that?

Comment 9

[gardan]

(In reply to Luca Greco [:rpl] from comment #7)

Thank you for your comment. If you have problem with rendering options twice, please see step 6 in post #3. It works ok for me, but it's still just workaround. I will try your npm package during the weekend.


(In reply to Dave Royal [:rpl] from comment #6)

Thanks Dave, it's really frustrating to see that the problem is reported for almost a year without any result. Hopefully things are going to change ... npm package from Luca is really good start)

Comment 10

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Dave Royal from comment #8)
> (In reply to Luca Greco [:rpl] from comment #7)
> Thank you.
> 
> I used the workaround on my addon
> https://addons.mozilla.org/en-US/firefox/addon/mudcat-browser-tools/
> The prefs now appear in fennec except for the control button:
>     "name": "readmeButton",   
>     "label": "ReadMe",    
>     "title": "ReadMe",        
>     "description": "What it does, preferences, caveats", 
>     "type": "control"
> The 'Readme' isn't visible and tapping it does nothing. 
> 
> The new version is here:
> https://dl.dropboxusercontent.com/u/862480/temp/mudcat_browse_tools%40davero.
> addons.mozilla.org-1.2.1.xpi
> 
> Can that be fixed? (It's not very important in my case!)

Thanks a lot for trying the workaround npm module, the above "button control issue" helped me to spot
another difference into the about:addons page on mobile and desktop firefox:

- on the desktop, the "about:addons" is a XUL page document
- on the mobile, the "about:addons" is a XHTML document 

to fix the issue (and probably other issues that we didn't notice yet) I've tweaked the workaround module to use
the XUL namespace when it creates the settings elements:

- https://github.com/rpl/fennec-addon-preferences-workaround/commit/a0fdd3f7b79ce299c2590ae45ba8190a41ac4f09#diff-168726dbe96b3ce427e7fedce31bb0bcR87

and I've added the "readmeButton" in the example addon and code that opens a tab when button is clicked (on both mobile and desktop Firefox):

- https://gist.github.com/rpl/7e009f3a504128605ae2feef13c95a29#file-index-js-L8
 
> Also, what's the significance of Fx 48? Is the problem fixed in that?

Once I've a full picture on the changes needed on 'native-options' module to make it work correctly on both mobile and desktop Firefox (and get the full picture is one of the goals of the above "workaround" npm module), then Fx 48 is the first version where I can land the change, move the fix into aurora and beta requires separate uplift requests.

Comment 11

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to gardan from comment #9)
> (In reply to Luca Greco [:rpl] from comment #7)
> 
> Thank you for your comment. If you have problem with rendering options
> twice, please see step 6 in post #3. It works ok for me, but it's still just
> workaround. I will try your npm package during the weekend.

Thanks for pointing it out, I'm using your workaround and it is very useful to spot the differences and plan the changes on native-options to make it work correctly.

Let me know how it goes with the updated workaround npm package applied to your use case, and feel free to "needinfo" me if you have any further issues, I'd really like to provide a reasonable solution which helps to make fix this issue as smooth as possible for the addon developers.

> 
> (In reply to Dave Royal [:rpl] from comment #6)
> 
> Thanks Dave, it's really frustrating to see that the problem is reported for
> almost a year without any result. Hopefully things are going to change ...
> npm package from Luca is really good start)

I completely understand the frustration, and introducing workarounds is always an annoying thing, 
I'm doing my best to make it "as less annoying as possible" ;-)

Comment 12

[Dave Royal]

(In reply to Luca Greco [:rpl] from comment #10)

The updated workaround works with both my addons that use fennec prefs. Thanks a lot :)

Comment 13

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Dave Royal from comment #12)
> The updated workaround works with both my addons that use fennec prefs.
> Thanks a lot :)
(In reply to gardan from comment #9)
> ... I will try your npm package during the weekend.

I've updated the workaround module based on AMO review comments (and other fixes and tweaks based on the patch I'm preparing for bug 1167246):

- https://github.com/rpl/fennec-addon-preferences-workaround/issues/1
- https://github.com/rpl/fennec-addon-preferences-workaround/pull/2

Comment 14

[Dave Royal]

Is the workaround compatible with the fix to bug 1167246 ?
i.e. will it continue to work in Fx48 or must it be removed?

Comment 15

[Dave Royal]

(In reply to Luca Greco [:rpl] from comment #13)
> 
> I've updated the workaround module based on AMO review comments (and other
> fixes and tweaks based on the patch I'm preparing for bug 1167246)

Before you did that I released an update to my Mudcat Browser Tools addon in comment 8. It's now the latest version 1.2.1 here:
https://addons.mozilla.org/en-US/firefox/addon/mudcat-browser-tools/
It works OK on fennec. The ReadMe button I mentioned in comment 8 is visible and tapping it opens the ReadMe page.

I updated that with the new version of the workaround (and some minor fixes) an produced version 1.2.2 which you will find here:
https://dl.dropboxusercontent.com/u/862480/temp/mudcat_browse_tools%40davero.addons.mozilla.org-1.2.2.xpi

The ReadMe button no longer works - tapping it does nothing.

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Dave Royal from comment #14)
> Is the workaround compatible with the fix to bug 1167246 ?
> i.e. will it continue to work in Fx48 or must it be removed?

Unfortunately if the workaround injects the preferences on a fixed version, the preferences
will be rendered twice.

My plan is to update the workaround module to auto-deactivate itself on Firefox 48.

(In reply to Dave Royal from comment #15)
> (In reply to Luca Greco [:rpl] from comment #13)
> > 
> > I've updated the workaround module based on AMO review comments (and other
> > fixes and tweaks based on the patch I'm preparing for bug 1167246)
> 
> Before you did that I released an update to my Mudcat Browser Tools addon in
> comment 8. It's now the latest version 1.2.1 here:
> https://addons.mozilla.org/en-US/firefox/addon/mudcat-browser-tools/
> It works OK on fennec. The ReadMe button I mentioned in comment 8 is visible
> and tapping it opens the ReadMe page.
> 
> I updated that with the new version of the workaround (and some minor fixes)
> an produced version 1.2.2 which you will find here:
> https://dl.dropboxusercontent.com/u/862480/temp/mudcat_browse_tools%40davero.
> addons.mozilla.org-1.2.2.xpi
> 
> The ReadMe button no longer works - tapping it does nothing.

Thanks a lot for testing and reporting it immediately!
I looked into it and it is due to a regression with the changes applied based on the AMO review comments,
it happens only when the button is not the last element of the array of preferences
and unfortunately the test suite didn't catched the regression (in the test case the button control was the last one in the defined preferences).

I'm working on fixing it in the native-options module in Bug 1263140, 
(and I'm going to fix it in the workaround module as well, asap)

Comment 17

[Luca Greco [:rpl] [:luca] [:lgreco]]

Updated 'fenned-addon-preferences-workaround' npm module:
- https://github.com/rpl/fennec-addon-preferences-workaround/commit/7c09a315956eed5d25060353e28dac5602a84e5a

This update should fix the regression on addon preferences button clicks and the workaround module should
be auto-disabled on Firefox for Android 48 where the fix on the SDK 'native-options' module is landed.

Hi Dave, let me know if this last update on the workaround module works correctly for you.

Comment 18

[Dave Royal]

That works fine. Thanks.

Comment 19

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562

Comment 20

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562