Add-ons manager doesn't display EULA for available add-ons that have one

VERIFIED FIXED in mozilla2.0b7

Status

()

Toolkit
Add-ons Manager
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

Trunk
mozilla2.0b7
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [strings])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
After searching AMO for add-ons if the user tries to install one with an EULA then we should display it for them to accept.

The XUL for the UI to do this still exists but a couple of strings got removed along the way so we'd need to add these back a.s.a.p.

There is probably also sadly a small API change (additive only) to support this.
(Assignee)

Updated

8 years ago
blocking2.0: --- → betaN+
(Assignee)

Comment 1

8 years ago
Blocking b7 for the strings at least.
blocking2.0: betaN+ → beta7+
(Assignee)

Updated

8 years ago
Assignee: nobody → dtownsend
(Assignee)

Comment 2

8 years ago
Created attachment 484381 [details] [diff] [review]
patch rev 1

Going to work on an automated test for this but I think we should try to get this reviewed and landed on trunk and relbranch a.s.a.p.

Turns out no API change was necessary, just needed to resurrect some strings from the 1.9.2 branch. I have not renamed the strings as they are exactly the same string in exactly the same context.
Attachment #484381 - Flags: review?(bmcbride)
(Assignee)

Comment 3

8 years ago
Created attachment 484382 [details] [diff] [review]
patch rev 1

Ignore the logging in the last patch
Attachment #484381 - Attachment is obsolete: true
Attachment #484382 - Flags: review?(bmcbride)
Attachment #484381 - Flags: review?(bmcbride)
Attachment #484382 - Flags: review?(bmcbride) → review+
(Assignee)

Comment 4

8 years ago
Landed: http://hg.mozilla.org/mozilla-central/rev/550280c5f30b
Relbranch: http://hg.mozilla.org/mozilla-central/rev/22a3c0fcb8f1
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite?
Flags: in-litmus-
Resolution: --- → FIXED
eulaHeader=%S requires that you accept the following End User License Agreement before installation can proceed:

What is %S? The add-on's name you're installing? Please add l10n comments ;-)
(Assignee)

Comment 7

8 years ago
Created attachment 484811 [details] [diff] [review]
testcase rev 1

Simple testcase, installs an extension from the results that requires accepting the EULA.
Attachment #484811 - Flags: review?(bmcbride)
(Assignee)

Comment 8

8 years ago
Comment on attachment 484811 [details] [diff] [review]
testcase rev 1

This isn't passing in conjunction with other tests right now
Attachment #484811 - Attachment is obsolete: true
Attachment #484811 - Flags: review?(bmcbride)
(Assignee)

Comment 9

8 years ago
Created attachment 485059 [details] [diff] [review]
testcase rev 2

Added some errors and cleanups at the end of every test when there are unexpected installs available and made the tests that violate this clean themselves up. This will help some of our intermittent failures not blow up the following tests quite so much.
Attachment #485059 - Flags: review?(bmcbride)
Comment on attachment 485059 [details] [diff] [review]
testcase rev 2

> function end_test() {
>   Services.prefs.clearUserPref("extensions.update.url");
> 
>-  finish();
>+  // Test generates a lot of available installs so just cancel them all
>+  AddonManager.getAllInstalls(function(aInstalls) {
>+    aInstalls.forEach(function(aInstall) {
>+      aInstall.cancel();
>+    });
>+
>+    finish();
>+  });
> }

Pity we can't have have async cleanup functions yet.

>+// Tests tha installs and undoing installs show up correctly

Typo.
Attachment #485059 - Flags: review?(bmcbride) → review+
(Assignee)

Comment 11

8 years ago
Landed the test: http://hg.mozilla.org/mozilla-central/rev/28675757affa
Flags: in-testsuite? → in-testsuite+
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101027 Firefox/4.0b8pre and the "Buzz It!" extension.

Btw. the monospace font we are using in this dialog is somewhat really ugly.
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla2.0b7
(In reply to comment #12)
> Btw. the monospace font we are using in this dialog is somewhat really ugly.

Going to fix that in bug 601022.
You need to log in before you can comment on or make changes to this bug.