Closed Bug 1494215 Opened Last year Closed Last year

Re-add xpinstallConfirm after its m-c removal in bug 1473933

Categories

(Thunderbird :: Add-Ons: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 64.0

People

(Reporter: Paenglab, Assigned: darktrojan)

References

Details

(Whiteboard: [Thunderbird-testfailure: Z][Thunderbird-disabled-test])

Attachments

(2 files, 1 obsolete file)

Bug 1473933 removes the xpinstallConfirm.xul which we are using to ask the user if the installation of a extension is okay.

Maybe instead of re-adding the files and somehow patch AddonManager.jsm, we could look to use the popup FX uses. In FX this popup appears in the URL bar.

Not tested, but probably until this is fixed, extensions can't be installed because we can't accept the installation.
Geoff, you already re-added the newAddon.xul, could you look into this? My JS experience is too low to fix this.
Flags: needinfo?(geoff)
You could also port whatever installation panel Firefox is using.
This is what I meant with the popup.
Causes test failures:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=201674639&revision=aaa4fd29eb4d6298ffd1d8e7828085b2706cbbbd

TEST-UNEXPECTED-FAIL | /builds/worker/workspace/build/tests/mozmill/content-tabs/test-install-xpi.js | test-install-xpi.js::test_install_xpi_offer
TEST-UNEXPECTED-FAIL | /builds/worker/workspace/build/tests/mozmill/content-tabs/test-install-xpi.js | test-install-xpi.js::test_xpinstall_actually_install
I've talked to Richard and Aceman on IRC. So instead of resurrecting the old xpinstallConfirm.xul/js/css/dtd/properties, we should port the stuff from comment #3. That will take a while, so let's disable the failing tests for now.
Assignee: jorgk → nobody
Keywords: leave-open
Whiteboard: [Thunderbird-testfailure: Z][Thunderbird-disabled-test]
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d9356caafc10
temporarily disable parts of MozMill test content-tabs/test-install-xpi.js. rs=bustage-fix
I'll take this. I'm not sure that the Firefox UI is going to fit well in Thunderbird, but I'll set that up first and then decide.
Assignee: nobody → geoff
Flags: needinfo?(geoff)
Attached patch 1494215-xpi-install-1.diff (obsolete) — Splinter Review
A good permanent fix is going to take some time to create. In the meantime I've moved the crucial parts to a notification, so that users can install add-ons again. (Not being able to do that is bad, even on Daily.)

I'll deal with the strings once I have some feedback.
Attachment #9014621 - Flags: feedback?(mkmelin+mozilla)
Attachment #9014621 - Flags: feedback?(jorgk)
Depends on: 1496632
Comment on attachment 9014621 [details] [diff] [review]
1494215-xpi-install-1.diff

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

Where am I supposed so see this? If I try to go to add-ons manager, select e.g. DKIM verifier, click Add to Thunderbird, it still just tries to download the xpi.
Comment on attachment 9014621 [details] [diff] [review]
1494215-xpi-install-1.diff

Perfect, I love it. Get it ready and landed :-)
Attachment #9014621 - Flags: feedback?(jorgk) → feedback+
Attachment #9014621 - Attachment is obsolete: true
Attachment #9014621 - Flags: feedback?(mkmelin+mozilla)
Attachment #9014976 - Flags: review?(jorgk)
Comment on attachment 9014976 [details] [diff] [review]
1494215-xpi-install-2.diff

Very nice. This the notification also shows up when you install manually via the menu or drag&drop, I've changed the string to:

Allow the installation of the following add-on in #1:;
Allow the installation of the following #2 add-ons in #1:

I couldn't work out how to trigger the plural message. Even when selecting two add-ons via the menu, I get two notifications, one on top of the other. Via drag&drop it only offers to install one. That's minor and most likely also doesn't work in FF.
Attachment #9014976 - Flags: review?(jorgk) → review+
Yes, that's interesting. Possibly a hangover from when you could install a XPI with several XPIs inside it (which might still be a thing, not sure) because I seem to recall that was the only way to have more than one extension listed in the old UI.

Anyway, it doesn't matter much. Ship it!
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/9528e42be304
Show notification to allow add-on installation after xpinstallConfirm removal in bug 1473933. r=jorgk
Status: NEW → RESOLVED
Closed: Last year
Keywords: checkin-needed
Resolution: --- → FIXED
(In reply to Geoff Lankow (:darktrojan) from comment #16)
> Ship it!
You bet!
Target Milestone: --- → Thunderbird 64.0
You need to log in before you can comment on or make changes to this bug.