Skip to content
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

Address compatibility issues with SeaMonkey 2.53.13 #1

Closed
wants to merge 1 commit into from

Conversation

UCyborg
Copy link

@UCyborg UCyborg commented Jul 17, 2022

Hi there, saw you in the recent issue of Palefill extension and also noticed that SeaMonkey's release notes on their official website point to your DownThemAll! extension repo for getting compatible version, which isn't compatible anymore since SeaMonkey version 2.53.13. I think I managed to address these incompatibilities.

Replaced legacy Iterator usage with Object.entries / Symbol.iterator.
Removed definition of programming language for "DownThemAll! connection" class.
Handle removed parameters of NSIInputStreamInput.input API.
Bumped up extension version and minimum versions of supported applications.
Also target Pale Moon, Iceape-UXP and Iceweasel-UXP.
Copy link
Owner

@dirkf dirkf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much!

A lot of work was done in the upstream master that isn't in the 3.0.9 code I worked on before. I finally spent time going through the changes and (not that I'm any kind of JS expert, and frankly the DTA code is quite challenging) I came to the conclusion that it would be better to build on that. However these changes are still required for SM 2.53.13 and are probably good for other UXP apps.

There are also test files that need updating; like this, I guess.

tests/manager/testImex.js:

 function imex_checkMetalinkExport(downloads, cb) {
 	metalink_getExportedResults(downloads, function(data, ex) {
-		for (var d in Iterator(downloads)) {
-			metalink_checkDownload(data.downloads, d[1]);
+		for (let [, d] of Object.entries(downloads)) {
+			metalink_checkDownload(data.downloads, d);
 		}
 		cb();
 	});
 }

tests/runner.js:

 	testHttpChannel.prototype = {
 		initializeTestChannel: function(opts) {
 			var requestHeaders = {}, responseHeaders = {};
-			for (var req in Iterator(opts.request)) {
-				requestHeaders[req[0].toLowerCase()] = req[1];
+			for (let [r, s] of Object.entries(opts.request)) {
+				requestHeaders[r.toLowerCase()] = s;
 			}
-			for (var res in Iterator(opts.response)) {
-				responseHeaders[res[0].toLowerCase()] = res[1];
+			for (let [r, s] of Object.entries(opts.response)) {
+				responseHeaders[r.toLowerCase()] = s;
 			}

tests/support/metalinkHelpers.js:

 			if (d.hash.pieces) {
 				hashCollection.parLength = d.hash.pieceLength;
-				for (var i in Iterator(d.hash.pieces)) {
-					hashCollection.add(new DTA.Hash(i[1], d.hash.pieceType));
+				for (let [,i] of Object.entries(d.hash.pieces)) {
+					hashCollection.add(new DTA.Hash(i, d.hash.pieceType));
 				}
 			}

@@ -474,7 +474,7 @@ var Servers = {
while (this._list.firstChild){
this._list.removeChild(this._list.firstChild);
}
for (let [,limit] in Iterator(this.listLimits())) {
for (let [,limit] of Object.entries(this.listLimits())) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the upstream master branch, Nils already updated this:

Suggested change
for (let [,limit] of Object.entries(this.listLimits())) {
for (let [,limit] of this.listLimits()) {

Apparently the line requireJoined(Servers, "support/serverlimits"); (see modules/glue.jsm:497) pulls in the definition of listLimits() method of Server, defined as the .entries() of a map object defined there. So the upstream version looks like it's OK. What do you think?

@@ -4,7 +4,7 @@
<em:id>{DDC359D1-844A-42a7-9AA1-88A850A938A8}</em:id>
<em:name>DownThemAll!</em:name>
<em:description>The mass downloader for Firefox.</em:description>
<em:version>3.0.9.2</em:version>
<em:version>3.0.9.3</em:version>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving to the upstream branch version:

Suggested change
<em:version>3.0.9.3</em:version>
<em:version>3.1.2pre</em:version>

Comment on lines +56 to +67
let pump;
try {
pump = new Instances.InputStreamPump(stream, SEGSIZE, SEGNUM, false);
}
catch (ex) {
if (ex.result === Cr.NS_ERROR_XPC_NOT_ENOUGH_ARGS) {
pump = new Instances.InputStreamPump(stream, 0, -1, SEGSIZE, SEGNUM, false);
}
else {
throw ex;
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frg noted this

Subject: Re: Will DownThemAll work on .13?
Date: Sun, 31 Jul 2022 23:38:10 +0200
From: frg frgrahl@gmx.net
Newsgroups: alt.comp.software.seamonkey
References: t8ik2s$sg0$1@gioia.aioe.org t8j8dd$19u2$1@gioia.aioe.org jh6csnFbukbU2@mid.individual.net tc4i7f$1pqv$1@gioia.aioe.org tc64jb$plj$1@gioia.aioe.org

I wrote:

On 31/07/2022 01:26, Jota.Ce wrote:

...>
I was delaying my update to .13 due to this thread i opened some time ago...

But no one has mentioned dTA is not working at all on .13 final.

...

I'm not running .13 yet but UCyborg has kindly submitted a PR
#1 for the necessary
changes (also note the new repo name). I need to get the build system working
again and that means syncing the build tree properly from the original legacy
master. So might take a couple of hours/days/weeks.

Didn't look at the whole js but for the Instances.InputStreamPump stuff I wonder if something like this would work too here:

https://hg.mozilla.org/chatzilla/rev/75c8c75015bc5afdf7930a834f599fe1c04f6eca

Checking the parameter could looks cleaner.

Maybe we need s/t like (is this right?):

		...
		const getInputStreamPump = function(stream, seg_size, seg_num, flag) {
			// see modules/glue.jsm:149 for prefix Plain to instantiate w/o init
			let pump = new Instances.PlainInputStreamPump();
			if (pump.init.length > 5) {
				pump.init(stream, 0, -1, seg_size, seg_num, flag);
			} else {
				pump.init(stream, seg_size, seg_num, flag);
			}
			return pump;
		}

		let pump = getInputStreamPump(stream, SEGSIZE, SEGNUM, false);
		pump.asyncRead(listener, null);
		...

And then use getInputStreamPump() again below.

Comment on lines +184 to +196
try {
new Instances.InputStreamPump(stream, SEGSIZE, SEGNUM, false).asyncRead(tee, null);
new Instances.InputStreamPump(pi, SEGSIZE, SEGNUM, true).asyncRead(listenerPartials, null);
}
catch (ex) {
if (ex.result === Cr.NS_ERROR_XPC_NOT_ENOUGH_ARGS) {
new Instances.InputStreamPump(stream, 0, -1, SEGSIZE, SEGNUM, false).asyncRead(tee, null);
new Instances.InputStreamPump(pi, 0, -1, SEGSIZE, SEGNUM, true).asyncRead(listenerPartials, null);
}
else {
throw ex;
}
}
Copy link
Owner

@dirkf dirkf Aug 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above:

Suggested change
try {
new Instances.InputStreamPump(stream, SEGSIZE, SEGNUM, false).asyncRead(tee, null);
new Instances.InputStreamPump(pi, SEGSIZE, SEGNUM, true).asyncRead(listenerPartials, null);
}
catch (ex) {
if (ex.result === Cr.NS_ERROR_XPC_NOT_ENOUGH_ARGS) {
new Instances.InputStreamPump(stream, 0, -1, SEGSIZE, SEGNUM, false).asyncRead(tee, null);
new Instances.InputStreamPump(pi, 0, -1, SEGSIZE, SEGNUM, true).asyncRead(listenerPartials, null);
}
else {
throw ex;
}
}
getInputStreamPump(stream, SEGSIZE, SEGNUM, false).asyncRead(tee, null);
getInputStreamPump(pi, SEGSIZE, SEGNUM, true).asyncRead(listenerPartials, null);

@dirkf
Copy link
Owner

dirkf commented Aug 27, 2022

Closed with f2f202f.

@dirkf dirkf closed this Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants