New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drag'n' drop into AOM: This add-on could not be installed because it appears to be corrupt #1663

Closed
arantius opened this Issue Oct 23, 2012 · 12 comments

Comments

Projects
None yet
3 participants
@arantius
Copy link
Collaborator

arantius commented Oct 23, 2012

Reported on the -users list:

Now I tried to reinstall these scripts again.
Therefore I clicked FF menu Tools--->Add-On--->User Scripts
and dragged some of the old *.user.js scripts onto FF.

But much to my surprise a popup bubble appears telling me:
"This add-on could not be installed because it appears to be corrupt"

Confirmed that this happens in the AOM but not a tab at a web page.

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Dec 20, 2012

The farthest I got today is that maybe if we registered part of the right category:
http://mxr.mozilla.org/mozilla-central/ident?i=CATEGORY_PROVIDER_MODULE
then we could end up in the list of addon providers:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm#524
and answer getInstallForURL() calls:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm#1271
when the url (/file) is a user script.

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jan 17, 2013

@janekptacijarabaci

This comment has been minimized.

Copy link
Contributor

janekptacijarabaci commented Jan 19, 2013

Maybe this will help:

greasemonkey_only

greasemonkey_and_scriptish

@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Jan 19, 2013

This seems to be caused by the hardcoded "application/x-xpinstall" MIME type in gDragDrop.onDrop (see in extensions.js), which is handled by XPIProvider. But since the file isn't actually an XPI, the XPIProvider throws when trying to unzip it, which triggers the "corrupt addon dialog".

Scriptish hooks into gDragDrop.onDrop to check the filename of dropped files against .user.js, but due to several bugs in that function it always returns early, effectively circumventing any special handling of drag/drop on about:addons - which results in both GM and Scriptish starting the normal installation process, as if the script was dropped on a "normal" tab (that is, any tab but about:addons).

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jan 19, 2013

Yeah, that calls AddonManager.getInstallForURL() with xpiinstall in the third argument. As I read the source, that just calls 'supportsMimetype' calls with that value, at least. But I don't see any. Maybe I'm reading something wrong, it's a lot of spaghetti callback stuff.

I'd like to avoid monkeypatching if there's a supported API. But might have to resort to it.

@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Jan 20, 2013

Yeah, the reason you're not seeing any calls is because the XPIProvider's callback is called first (probably because it registered earlier or something), and getInstallForURL returns after it found the first addon provider that accepts the passed MIME type. So basically the call chain is:

AddonManager.getInstallForURL -> ... -> XPIProvider.supportsMimetype (returns true) -> XPIProvider.getInstallForURL (which throws and triggers the dialog) -> end

whereas the correct one would be

AddonManager.getInstallForURL -> ... -> XPIProvider.supportsMimetype (returns false) -> GM's AddonProvider.supportsMimeType (returns true) -> etc.

So it looks like unless upstream changes gDragDrop.onDrop to pass the correct MIME type, monkey patching is the only option...

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jan 24, 2013

... and getInstallForURL returns after it found the first addon provider that accepts the passed MIME type ...

Ah, absolutely. Somewhere else (I did a lot of back and forth trying to trace this) there was some sort of async-callback iterator thing; I missed that this was a for..let here, which definitely has that behavior.

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jan 24, 2013

If we call the original dragdrop handler, then we get the 'corrupt' notice. If we don't (when there is a .user.js) call it then we don't, but this would break dropping one .xpi and one .user.js (or other combinations with both).

When there is any .user.js file dropped, should we call the original handler?

@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Jan 24, 2013

I think the original handler should still be called, however the DataTransfer object should have the data for the corresponding index removed (i.e. if dataTransfer.mozGetDataAt(mimeType, i) returns a user script file/URL, dataTransfer.mozClearDataAt(mimeType, i) should be called before calling the original handler).

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jan 24, 2013

Trying to .mozClearDataAt() gives

Timestamp: 01/24/2013 12:57:41 PM
Error: NoModificationAllowedError: Modifications are not allowed for this document
Source File: chrome://greasemonkey/content/addons4-overlay.js
Line: 39
@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Jan 24, 2013

Mhh, indeed, see this comment on Mozilla's bugzilla. Not sure how to handle it then, other than not calling the original handler when at least one userscript is dropped onto the page. But even that would be better than not handling userscripts at all - especially because I don't think it happens very often that someone tries to drop an XPI and a userscript at the same time.

arantius added a commit to arantius/greasemonkey that referenced this issue Jan 24, 2013

@arantius arantius closed this in 819117a Jan 24, 2013

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jan 24, 2013

I picked calling the default handler when and if there is at least one non- user script; this will still throw the message, but should install both, if a .xpi and a .user.js are both dragged at the same time, and should handle any number of either properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment