Closed Bug 605499 Opened 9 years ago Closed 9 years ago

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

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: mossop, Assigned: mossop)

Details

(Whiteboard: [strings])

Attachments

(2 files, 2 obsolete files)

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.
blocking2.0: --- → betaN+
Blocking b7 for the strings at least.
blocking2.0: betaN+ → beta7+
Assignee: nobody → dtownsend
Attached patch patch rev 1 (obsolete) — Splinter Review
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)
Attached patch patch rev 1Splinter Review
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+
Landed: http://hg.mozilla.org/mozilla-central/rev/550280c5f30b
Relbranch: http://hg.mozilla.org/mozilla-central/rev/22a3c0fcb8f1
Status: NEW → RESOLVED
Closed: 9 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 ;-)
Attached patch testcase rev 1 (obsolete) — Splinter Review
Simple testcase, installs an extension from the results that requires accepting the EULA.
Attachment #484811 - Flags: review?(bmcbride)
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)
Attached patch testcase rev 2Splinter Review
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+
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.