Closed Bug 1329450 Opened 3 years ago Closed 3 years ago

Add-on install snackbar/permission popup missing

Categories

(Firefox for Android :: Add-on Manager, defect, P1)

53 Branch
All
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 53
Tracking Status
fennec + ---
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- verified

People

(Reporter: JanH, Assigned: aswan)

References

Details

(Keywords: regression)

Attachments

(1 file)

Seems like Fennec was forgotten when amIWebInstaller was removed in bug 1323129, which has broken
- the snackbar that appears when downloading an add-on
- and more importantly, the permissions dialogue that pops up when the install was blocked

https://dxr.mozilla.org/mozilla-central/rev/0d823cf54df53e0cea75a74adebace956bd333d8/mobile/android/chrome/content/browser.js#5163

> [JavaScript Error: "TypeError: invalid 'instanceof' operand Ci.amIWebInstallInfo" {file: "chrome://browser/content/browser.js" line: 5161}]
> observe@chrome://browser/content/browser.js:5161:1
> installNotifyObservers@resource://gre/modules/AddonManager.jsm:460:3
> Installer@resource://gre/modules/AddonManager.jsm:482:5
> onWebInstallRequested@resource://gre/modules/AddonManager.jsm:669:5
> installAddonFromWebpage@resource://gre/modules/AddonManager.jsm:2311:9
> installAddonFromWebpage@resource://gre/modules/AddonManager.jsm:3639:5
> installAddonFromWebpage/<@jar:jar:file:///data/app/org.mozilla.fennec-1/base.apk!/assets/omni.ja!/components/addonManager.js:128:7
> safeCall@resource://gre/modules/AddonManager.jsm:191:5
> makeSafe/<@resource://gre/modules/AddonManager.jsm:206:25
> process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:917:23
> walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:801:7
> scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> re
Flags: needinfo?(aswan)
Fennec wasn't forgotten, I assumed a clean try run would be sufficient.
From quick inspection, this ought to be fixable with something like the inline patch below.
I tried to read the instructions about how to run Fennec in an emulator from here but the external links are 404s: https://wiki.mozilla.org/Mobile/Fennec/Android/Emulator

Jan, do you want to try out the patch below or if not, can you point me to current instructions for running an emulator (on a mac)?

diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -5155,18 +5155,18 @@ var XPInstallObserver = {
     Services.obs.addObserver(this, "xpi-signature-changed", false);
     Services.obs.addObserver(this, "browser-delayed-startup-finished", false);
 
     AddonManager.addInstallListener(this);
   },
 
   observe: function(aSubject, aTopic, aData) {
     let installInfo, tab, host;
-    if (aSubject && aSubject instanceof Ci.amIWebInstallInfo) {
-      installInfo = aSubject;
+    if (aSubject && aSubject.wrappedJSObject) {
+      installInfo = aSubject.wrappedJSObject;
       tab = BrowserApp.getTabForBrowser(installInfo.browser);
       if (installInfo.originatingURI) {
         host = installInfo.originatingURI.host;
       }
     }
 
     let strings = Strings.browser;
     let brandShortName = Strings.brand.GetStringFromName("brandShortName");
Flags: needinfo?(aswan) → needinfo?(jh+bugzilla)
Sure, I can give this a try later.

If you want to look into it yourself, https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build should most of - I think that if you try ./mach install or ./mach run without a physical Android device attached, it'll automatically attempt to start the emulator (otherwise use ./mach android-emulator first). I can't guarantee it's 100% bug-free at the moment though. If in doubt, better ask in #mobile ...
Yup, seems working fine for me.
Flags: needinfo?(jh+bugzilla)
Alright.  Sorry to continue to display my Fennec ignorance but do you think this is something we could/should add a test for or should we just land this as-is and move on?
Flags: needinfo?(jh+bugzilla)
I'm no expert there, but

> could
Probably yes - because of the interaction with the native UI on Android, it might have to be a Robocop test (https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/robocop/README.rst) to check that the expected UI is displaying and to cancel the attempted add-on installation.

> should
It would of course be nice, but you won't get shouted at if you didn't and just left a follow-up bug - I might be able to pick it up (although probably not very soon) or maybe somebody else would.
Flags: needinfo?(jh+bugzilla)
tracking-fennec: ? → +
Priority: -- → P1
Duplicate of this bug: 1330537
Attachment #8826617 - Flags: review?(jh+bugzilla)
Assignee: nobody → aswan
Comment on attachment 8826617 [details]
Bug 1329450 Fix amIWebInstallInfo fallout on fennec

https://reviewboard.mozilla.org/r/104544/#review105300
Attachment #8826617 - Flags: review?(jh+bugzilla) → review+
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9cb19aceacf0
Fix amIWebInstallInfo fallout on fennec r=JanH
https://hg.mozilla.org/mozilla-central/rev/9cb19aceacf0
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
This issue is verified as fixed on Firefox 53.0a1 (2017-01-15) under Android 6.0. 
The error presented in the description of this bug is not displayed anymore in remote debugging console and the add-ons can be installed without any issues.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.