[RESOLVED] FlashGot not working in Firefox Nightly 64

[Atenza]

Hi

When I use Flashgot it makes files with 0 byte and nothing else. I am trying to use default download manager. This works with other things.

[Giorgio Maone]

Could you show me your FlashGot Options|Advanced|Show log output?

[Giorgio Maone]

Thanks for submitting your log.
Looks like a fallout of this recent huge and still under development Firefox download manager overhaul.
I'm trying to fix that ASAP.

[user]

They should try harder, because I still was able to trace the error source despite the messed-up stacktrace.

Code: Select all

2013-09-10T22:15:10.420Z Skipping link https://download-installer.cdn.mozilla.net/pub/mozilla.org/firefox/releases/23.0.1/win32/en-US/Firefox%20Setup%20Stub%2023.0.1.exe: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDownloadManager.addDownload]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://flashgot/content/DMS.js :: FlashGotDM.initDMS/dm.performDownload/mozAddDownload :: line 2551"  data: no]

From the docs:

Warning: This interface is deprecated as of Firefox 26. Please use Downloads.jsm instead.

[Giorgio Maone]

They should try harder, because I still was able to trace the error source despite the fucked up stacktrace.

Finding the problem was not hard (I pointed the relevant docs in my previous post).
Refactoring the code to use the new unstable API when needed (i.e. in the newest Nightlies and only if a soon to be gone preference is set to true) while keeping backward compatibility with another 4 (four) previous APIs requires a bit more care and time, though.

[Atenza]

Hi

Will there be an update of FlashGot for this problem ?

[Giorgio Maone]

Will there be an update of FlashGot for this problem ?

I'm trying to fix that ASAP.

[user]

Try this:

Code: Select all

http://www.putlocker.com/file/985FEC33BF45A95A - click "Continue as free user", not "Download now".
http://www17.zippyshare.com/v/40150093/file.html

[Giorgio Maone]

Try this:

Code: Select all

http://www.putlocker.com/file/985FEC33BF45A95A - click "Continue as free user", not "Download now".
http://www17.zippyshare.com/v/40150093/file.html

Darn good job!
I was half way, leaving the POST support temporarily out because Mozilla folks should really build it in a saner way than forcing clients to JSON-serialize potentially huge payloads.
Anyway, I'm merging your code, thank you again.

[Giorgio Maone]

Try this:

Code: Select all

http://www.putlocker.com/file/985FEC33BF45A95A - click "Continue as free user", not "Download now".
http://www17.zippyshare.com/v/40150093/file.html

Darn good job!

@user: BTW, are all those Components.isSuccessCode() checks you added in MediaSniffer meant to skip on internal redirections and fix the Youtube extra loads bug?

[user]

The payloads won't be huge because we're talking about _down_loads, not _up_loads. The biggest payload I've seen was on some filehosting: about 2 KB, 10+ hidden form fields. But that's an "extreme" case, usually it'll be even less - just a download/request/transaction ID and/or a disposable request token to prevent request replay. For instance, Grooveshark's payload is about 100 bytes.

As for Components.isSuccessCode() calls, call it my extra paranoia. The one in onStateChange() is for cases when the connection is interrupted before the response headers arrive. I such cases, accessing the channel properties like responseStatus or contentType will throw and pollute the log. (The only easy way to reproduce that case I can come up with is to refresh the page repeatedly by holding F5/Ctrl+R for a second or two.) I think the one in the beginning of observe() can be safely dropped because if I'm not missing anything, observe() is called either by the browser (who, I think, won't call it for an incomplete/interrupted response) or by MediaSniffer::onStateChange.

And before you merge: there's another comment (DMS.js, line 2566) that AMO auditors probably won't like. Although I don't understand why do you care about their likes. I thought their job is auditing the code, not approving the comments.

[Giorgio Maone]

The payloads won't be huge because we're talking about _down_loads, not _up_loads. The biggest payload I've seen was on some filehosting: about 2 KB, 10+ hidden form fields. But that's an "extreme" case, usually it'll be even less - just a download/request/transaction ID and/or a disposable request token to prevent request replay. For instance, Grooveshark's payload is about 100 bytes.

That's what I guessed too, but still handling POST-triggered download is required as a built-in feature of Firefox's download manager, and in the general case you can't rely on website to play nice or use common sense (a huge upload which gets serialized in memory as a JSON string could be used as a DOS attack).

I think the one in the beginning of observe() can be safely dropped because if I'm not missing anything, observe() is called either by the browser (who, I think, won't call it for an incomplete/interrupted response) or by MediaSniffer::onStateChange

I choose to drop the one in onStateChange() instead, because as far as I can see it just calls observe() which is already guarded.
As for the observers not being called on non-success codes, I'd prefer to err on the safe side.

And before you merge: there's another comment (DMS.js, line 2566) that AMO auditors probably won't like. Although I don't understand why do you care about their likes. I thought their job is auditing the code, not approving the comments.

I already noticed and removed it, thanks. As much as I share your feelings about breaking stuff by fixing what's not broken, I'm part of that community too, and generally prefer to discuss such issues politely (like in "open a bug report", as I did) rather than creating hard feelings which don't get things fixed and alienate collaborative attitudes

[user]

I choose to drop the one in onStateChange() instead, because as far as I can see it just calls observe() which is already guarded.

No, I lied, we need both. In onStateChange, the reason I check the |status| parameter and not channel.status is that the latter can be zero (which means success) even if |status| is not. Just try that repeated refresh thing and you'll see.

[Giorgio Maone]

Please check latest development build 1.5.5.9rc1, thank you.

[Razlo]

Please check latest development build 1.5.5.9rc1, thank you.

Negative. Tried installing it twice and still not outputting the data past the header.