892388 - Create a clone of restartless.xpi for uploading to AMO with an end user license agreement

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

In case we may ultimately want to have the addon uploaded without the EULA, we should clone it and make the name/id distinct.

I would suggest:
id: [e-mail]
name: Restartless Addon with EULA

Comment 1

[mario garbi]

Attachment: patch v1.0

The binary patch for the restartless.xpi clone named restartless-eula.xpi .

Comment 2

[mario garbi]

Attachment: patch v1.1

Made a few fixes that I overlooked in the first patch. I have also updated the Description and addon version (3.2)

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 773848 [details] [diff] [review]
patch v1.1

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

There's an extra install.rdf~ file included. Please remove this and repackage.

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Let's also confirm with Henrik what details we want this addon to have. The following is extracted from the install.rdf in the latest patch:

id: [e-mail]
version: 3.2
name: Restartless Addon with EULA
description: Restartless Addon with EULA to be used in tests

Comment 5

[mario garbi]

Attachment: patch v1.2

I removed the backup file that I overlooked on the last patch and repackaged the addon.

Comment 6

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

(In reply to Dave Hunt (:davehunt) from comment #4)
> id: [e-mail]
> version: 3.2
> name: Restartless Addon with EULA
> description: Restartless Addon with EULA to be used in tests

Sounds fine to me.

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 774506 [details] [diff] [review]
patch v1.2

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

Thanks. Landed as:
https://hg.mozilla.org/qa/testcase-data/rev/d2bbf1115cd5

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I've had to backout this change because the install.rdf was not able to be parsed by AMO. Please test before providing a new patch.

https://hg.mozilla.org/qa/testcase-data/rev/3ebc19f774e9

Comment 9

[mario garbi]

Attachment: patch v1.3

Fixed the issues with AMO, now the addon can be uploaded without problems on AMO. Also reverted back to version 3.1 as I talked to Dave on IRC.

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 774519 [details] [diff] [review]
patch v1.3

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

Landed as:
https://hg.mozilla.org/qa/testcase-data/rev/b6700a219701

Submitted to AMO as:
https://addons.mozilla.org/en-US/firefox/addon/restartless-addon-with-eula/

Comment 11

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

Can we close this bug?

Comment 12

[mario garbi]

We could wait until the addons is approved on AMO, other than that I'm not sure what else could be done here.

Comment 13

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

No reason to keep that open then. Closing as fixed! Thanks Mario.

Comment 14

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

Review has been denied due to:

1) You are using an outdated version of the Add-ons SDK, which we no longer accept. Please repack your code with the latest stable version of the SDK: https://addons.mozilla.org/en-US/developers/builder

So please build the XPI against the latest SDK!

Comment 15

[mario garbi]

Attachment: restartlessEULA_2307.patch

Repackaged the addon with the latest SDK and tested if it's accepted for upload on AMO. For the Firefox versions I've left them as they were autogenerated and it works ok with both ESR17 and Nightly 25 without any problems.

Comment 16

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 779677 [details] [diff] [review]
restartlessEULA_2307.patch

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

This patch will not apply. Did you pull in the latest changes before creating the patch?

$ hg qpush
applying bug892388.patch
file firefox/addons/extensions/restartless-eula.xpi already exists
1 out of 1 hunks FAILED -- saving rejects to file firefox/addons/extensions/restartless-eula.xpi.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh bug892388.patch

Comment 17

[mario garbi]

Attachment: restartlessEula_2307.patch

Sorry about that and thank you for the fast review. I got too focused on repackaging the addon and overlooked something as basic as updating the testcase-data.

Comment 18

[mario garbi]

Setting dependencies.

Comment 19

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 779721 [details] [diff] [review]
restartlessEula_2307.patch

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

Can you detail what you did in this patch? I only looked briefly over the files and noticed a couple of issues, listed below:

1. The version number has not been updated. As we've updated the SDK we should definitely bump the version number.
2. The minVersion has gone from 4.0 to 19.0
3. The maxVersion has dropped from 25.0 to 20.*

Comment 20

[mario garbi]

I've left the version to 3.1 as you told me back then on skype, the minVersion doesn't really matter when installing the addon as it works with esr17 even with minVersion set at 19. They were set by default when I repackaged the addon with the latest SDK and I figured I shouldn't change them if they worked on all firefox builds.

Basically what I did here was repackage the entire addon using the latest SDK so it would be approved on AMO as Henrik said.

Should I modify the min/maxVersion values and increment the version?

Comment 21

[mario garbi]

Attachment: restartlessEula_1208.patch

Made the changes requested and uploaded the new addon patch. I've tested it on AMO and it's accepted for upload.
Version now incremented to 3.2 and min/max values updated to 4.0-25.*

Comment 22

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to mario garbi from comment #20)
> I've left the version to 3.1 as you told me back then on skype

We haven't spoken on Skype about this. Previously we wanted to keep the version number the same when it was just a fork of the non-eula addon, but now we're rebuilding it based on a new SDK version, so we should bump the version number.

> minVersion doesn't really matter when installing the addon as it works with
> esr17 even with minVersion set at 19. They were set by default when I
> repackaged the addon with the latest SDK and I figured I shouldn't change
> them if they worked on all firefox builds.

If it works on ESR 17 then we should at least set minVersion to 17.0 as this is the lowest version number we run automation on.

> Basically what I did here was repackage the entire addon using the latest
> SDK so it would be approved on AMO as Henrik said.

Can you provide more details, as requested?

> Should I modify the min/maxVersion values and increment the version?

Yes, please.

Comment 23

[mario garbi]

I have already uploaded an updated version with minVersion set to 4.0 again. Sorry about the "skype" thing, I was thinking IRC while talking on skype and my mind must have made some weird connections.

Comment 24

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

Comment on attachment 788914 [details] [diff] [review]
restartlessEula_1208.patch

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

Some things I have noticed:

* We should not open a special page on mozqa.com but https://www.mozqa.com/data/
* The creator of the extension should be Automation Development - but we might want to spin off a new bug to update all add-ons
* As FQDN for the extension we might want to use ´mozqa.com´ now. So we could name it [e-mail]

Comment 25

[mario garbi]

Attachment: restartlessEula_1908.patch

I have tested that the patch is accepted on AMO and that it's functional on firefox. With this patch I made the required changes if I understood them correctly.

"<em:id>[e-mail]</em:id>"
"<em:creator>Automation Development</em:creator>"
"var menuItem = contextMenu.Item({
    label: "Open mozqa.com",
    contentScript: 'self.on("click", function (node, data) {' +
                   ' window.location.href = "http://mozqa.com/data/";' +
                   '});' "

If there is something I missed or misunderstood I would be happy to fix it as soon as possible.

Comment 26

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

Comment on attachment 792131 [details] [diff] [review]
restartlessEula_1908.patch

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

That looks good now. Lets get it finally in:
http://hg.mozilla.org/qa/testcase-data/rev/0f3866a9f9ea

Comment 27

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

Uploading the add-on to AMO was not a good idea. I can't do it myself anymore. So the only possible way I can see is to get it transferred to the https://addons.mozilla.org/en-US/firefox/user/qa-auto/ account. The email address to use I can give you tomorrow.

Comment 28

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Henrik Skupin (:whimboo) from comment #24)
> * As FQDN for the extension we might want to use ´mozqa.com´ now. So we
> could name it [e-mail]

This has been updated in install.rdf but not in harness-options.json, did you rename it after building the extension? This failed validation when I attempted to upload it to AMO.

(In reply to Henrik Skupin (:whimboo) from comment #27)
> Uploading the add-on to AMO was not a good idea. I can't do it myself
> anymore. So the only possible way I can see is to get it transferred to the
> https://addons.mozilla.org/en-US/firefox/user/qa-auto/ account. The email
> address to use I can give you tomorrow.

I'm not sure I follow you Henrik... I've been using qa-auto to upload this addon. The latest issue was that you requested the ID to be changed in comment 24, which you can't change once you've created an AMO submission. I have deleted the original incomplete submission so this shouldn't be a problem any more.

Comment 29

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

(In reply to Dave Hunt (:davehunt) from comment #28)
> (In reply to Henrik Skupin (:whimboo) from comment #24)
> > * As FQDN for the extension we might want to use ´mozqa.com´ now. So we
> > could name it [e-mail]
> 
> This has been updated in install.rdf but not in harness-options.json, did
> you rename it after building the extension? This failed validation when I
> attempted to upload it to AMO.

harness-options.json is auto-generated by the cfx tool. So it should pick up any changes from the install.rdf. Whenever the rdf changes you really have to run the cfx tool again. Did you miss that, Mario?

> (In reply to Henrik Skupin (:whimboo) from comment #27)
> I'm not sure I follow you Henrik... I've been using qa-auto to upload this
> addon. The latest issue was that you requested the ID to be changed in
> comment 24, which you can't change once you've created an AMO submission. I
> have deleted the original incomplete submission so this shouldn't be a
> problem any more.

Oh, by that time Mario uploaded the add-on to his account. So the id was also taken. Might be that he deleted it meanwhile, and we are now seeing a different issue.

I would really love to get this in finally. It takes way too long, sorry.

Comment 30

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Henrik Skupin (:whimboo) from comment #29)
> Oh, by that time Mario uploaded the add-on to his account. So the id was
> also taken. Might be that he deleted it meanwhile, and we are now seeing a
> different issue.

Let's please use qa-auto and not someone's personal account. If it still exists on your account could you please remove it Mario?

Comment 31

[mario garbi]

I haven't uploaded restartless-eula.xpi to AMO. I was checking that it was accepted by AMO but never really finished uploading.
I tried again to upload it and I only get one warning about "Jetpack module hash mismatch". I haven't run the cfx tool after making the changes, still learning how addon compile works. I will fix it asap.

Comment 32

[mario garbi]

Attachment: restartlessEula_2508.patch

Rebuilt the .xpi with the correct information by providing it directly into the package.json before triggering 'cfx xpi' and only modified install.rdf for the firefox versions (17.0-25.*).
I checked it on AMO (without finishing upload) and we get no warnings or notifications so this should be ok now. First time working with Addons SDK and sorry for the previous mistakes.

Comment 33

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

I haven't expected an update for this bug given that there are other more important (P1) bugs to work on.

Comment 34

[mario garbi]

I was investigating the P1 issues on the Mac machine and while checking for the regression range I was able to rebuild this on the Linux machine. The P1 bugs have priority of course.

Comment 35

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

Comment on attachment 795412 [details] [diff] [review]
restartlessEula_2508.patch

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

That finally looks fine to me. Sadly I'm not able to upload the extension to AMO given that the validation process never finishes. Dave, can you try it please?

Comment 36

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I'm also unable to upload this at the moment. First, it seemed to halt at 91% complete - 158.37 KB of 174.03 KB then I got the following message:

Your add-on failed validation with 1 error.

    Received an empty response from the server; status: 0

Comment 37

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

Add-on has been submitted to AMO:
https://addons.mozilla.org/en-US/firefox/addon/restartless-addon-with-eula/