785014 - Build restartless.xpi as a plain bootstrapped extension

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Infrastructure, defect, P2)

Description

[Maniac Vlad Florin (:vladmaniac)]

We need to update restartless.xpi test add-on because its causing one of our mozmill tests to fail

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This has to be build as a plain bootstrapped extension *without* making use of the SDK. I have expressed that in the referenced bug. It's important because the Addon SDK offers compatibility only for the current and next version of Firefox. You can see MDN how to create such an extension. 

Alex or Vlad, can one of you take this?

Comment 2

[Alex Lakatos[:AlexLakatos]]

I'll take it since I'm the original creator of restartless.xpi.

Comment 3

[Andreea Matei [:AndreeaMatei]]

Alexandre, could you please tell me if I'm going on the right path here?

I have worked so far with this document for the bootstrap.js part:
https://developer.mozilla.org/en-US/docs/Extensions/Bootstrapped_extensions

We need our bootstrapped addon to add a context menu item labeled "Open mozqa.com" which when called, should open a new tab, loading mozqa.com page. The current addon [1] doesn't work anymore due to changes here:
http://hg.mozilla.org/mozilla-central/rev/478ef0049c1a

Regarding the context menu part, not sure if is something different that has to be done instead of what we have in our current restartless Addon (in resources we have a lib context-menu.js that we use to add the new item, which is now failing:

  var menuItem = contextMenu.Item({
    label: "Open mozqa.com",
    contentScript: 'self.on("click", function (node, data) {' +
                   ' window.location.href = "http://mozqa.com/data/firefox/layout/mozilla.html";' +
                   '});'

I found some documentation, but only from Mobile:
https://developer.mozilla.org/en-US/docs/Extensions/Mobile/Addons_developer_guide

[1] http://www.mozqa.com/data/firefox/addons/extensions/restartless.xpi

Comment 4

[Alexandre Poirot [:ochameau]]

Did you really meant to ask me for info?
I'm kind of lost in this bug...

The only thing I can say is Henrik asked for a non-jetpack addon, and http://www.mozqa.com/data/firefox/addons/extensions/restartless.xpi is a jetpack addon.

Comment 5

[Alexandre Poirot [:ochameau]]

If you want to implement a context menu without the SDK, you can give a look at this page:
https://developer.mozilla.org/en-US/docs/XUL/PopupGuide/ContextMenus

But it will be quite challenging to do that in a restartless addon, especially if you don't have lot of experience with addons.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Alexandre Poirot (:ochameau) from comment #4)
> The only thing I can say is Henrik asked for a non-jetpack addon, and
> http://www.mozqa.com/data/firefox/addons/extensions/restartless.xpi is a
> jetpack addon.

Thing is that we need an extension which is compatible with all versions of Firefox. Given the nature of the SDK we can't guarantee that, right? Also not sure how things have changed since the code is in Firefox core now.

One thing we need for the extension is any kind of ui, it doesn't matter what, so we can prove it is installed and performs an action. Would you have another idea what we could do beside a context menu? Probably something more stable?

Comment 7

[Alexandre Poirot [:ochameau]]

I have now no clear idea how things are going to evolve regarding firefox UI with the upcoming australis project and how the SDK is going to support it.
The fact that SDK being in firefox, it will prevent you from having to rebuild the xpi every now and then in order to update sdk modules to work with new firefox versions. But then, you have to ensure that the SDK UI module (like widget, context-menu and so on) you are going to use will be ensured to always work. Given the australis story, I think that I heard talking about deprecating the Widget API. (I may be wrong as I left the SDK team) In the worse scenario, you will just have to update your addon when australis land. (No matter what kind of addon your are doing, you might have to tweak your addon for australis...)

Matteo is the SDK UI specialist. He may have a good insight about this.

Comment 8

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

Alex is right: we're going to deprecate Widget API when Australis will land – and therefore developers will have the new UI SDK to use.
Of course the deprecation process will takes time, so you will still have Widget API available in order to safely move to the new APIs.
We're not going to remove the Widget API until we didn't implement the UI API completely.

I'm not totally sure what is the context here, but what I would if I need a UI across Firefox SDK versions, will be use Widget API in non Australis Firefox, and new Button API in Australis Firefox. You can also share the same "options" to pass to the constructor – more or less.

But yes, as Alex said you have to update your addon when australis land.

Comment 9

[Teodor Druta]

As discussed on IRC, this is not required anymore, will mark it as WONTFIX.

Comment 10

[Teodor Druta]

This is still actually required for Bug 784305.
So, we need a simple bootstrap extensions which will add a new entry in the context menu with the functionality of a link to mozqa (eg. http://mozqa.com/data/firefox/layout/mozilla.html).

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Or we update the extension to work with the new SDK. This might actually be faster, but could require updates later. I wouldn't worry about that for now. Lets do here what is faster, so that we can get the test re-enabled.

Comment 12

[Teodor Druta]

Attachment: restartless_addon.patch

The old extension was too out-dated, modifying such a small extension in functionality would take a lot of time (I tried that).
The new addons sdk is actually pretty neat and well documented.
This extension will add a new Entry in the Page content context menu, name Open MozQA, selecting it will open a new tab to "http://mozqa.com/data/firefox/layout/mozilla.html".
Also fixed the test at Bug 784305. So this patch should be landed first.

Comment 13

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8554503 [details] [diff] [review]
restartless_addon.patch

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

No checkin flag needed yet.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8554503 [details] [diff] [review]
restartless_addon.patch

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

Please add a real patch and not the XPI. I would suggest that we use a sub-folder called addons/src/extension_id where we can save off the code, and put the XPI under addons/extensions

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Or skip that for now and we do that later. It will save us some time. I will have a look at the extension now.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8554503 [details] [diff] [review]
restartless_addon.patch

This is not a patch against testcase-data when it has to be landed first, and this bug is about.

Comment 17

[Teodor Druta]

(In reply to Henrik Skupin (:whimboo) from comment #16)
> Comment on attachment 8554503 [details] [diff] [review]
> restartless_addon.patch
> 
> This is not a patch against testcase-data when it has to be landed first,
> and this bug is about.

ok, then I'll upload a patch for testcase-data as well, but can you please take a look over the code and provide some feedback ?

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Once the patch is up I can do.

Comment 19

[Teodor Druta]

Attachment: restartless_extension.patch

This is the patch for the testcase-data repository.
One thing I'd like to notice here is the difference in size between the new extension and the original restartless add-on, the old one was ~160kB the new one is around 7kB.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8555724 [details] [diff] [review]
restartless_extension.patch

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

The changes look fine and I'm happy about the code reduction! There are only some nits to be updated... Please keep the install.rdf details from the former extension as much as possible (id, creator, etc.)

Comment 21

[Teodor Druta]

Attachment: restartless_extension.patch

Updated the package.json values to correspond with those of install.rdf from the old extension.
Note: min/max version are automatically generated by cfx tool this cannot really be changed except by editing directly the install.rdf, this will not result in any compatibility issues with different firefox versions, it's just used for the addons.mozilla.org page.

Comment 22

[Teodor Druta]

Attachment: restartless_src.tar.bz2

... and here are the _src files, for future references/possible updates.
Please refer to the official add-on sdk and cfx tools documentation:
https://developer.mozilla.org/en-US/Add-ons/SDK/Tutorials/Installation
https://developer.mozilla.org/en-US/Add-ons/SDK/Tools/cfx

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8556396 [details] [diff] [review]
restartless_extension.patch

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

I gave my feedback already, so not sure why it has another round. Lets Andreea review it and shipped if she is ok.

Comment 24

[Andreea Matei [:AndreeaMatei]]

I'm waiting for a confirmation from Teodor that this was tested with the test in bug 784305 on all platforms.

Comment 25

[Teodor Druta]

Comment on attachment 8556396 [details] [diff] [review]
restartless_extension.patch

It's all good, the extension works with all platforms and branches.

Comment 26

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8556396 [details] [diff] [review]
restartless_extension.patch

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

Great then, I added it:
https://hg.mozilla.org/qa/testcase-data/rev/215ce8221272 (testcase-data)

We need it in the other bug for mozmill-tests too.