Remove references to XMLHttpRequest.channel

[mjethani]

Background

adblockpluscore still uses internal Gecko APIs (#5702) via wrappers. In practice they do nothing. One of these APIs is ​XMLHttpRequest.channel. We create a mock version of this API in the WebExt version of Adblock Plus, but this is totally unnecessary now since the legacy extension is no longer supported.

What to change

In lib/downloader.js, stop referring to the channel property of the XMLHttpRequest object. Modify test/_common.js as well as any other test files to stop using this API.

Integration notes

After this change, there's no need for nsIHttpChannel and XMLHttpRequest.prototype.channel in lib/compat.js in adblockpluschrome; these should be removed.

Also, in adblockpluschrome, remove &channelStatus=%CHANNELSTATUS% from defaults.subscriptions_fallbackurl in lib/prefs.js.

Hints for testers

Make sure downloadable subscriptions are synced correctly. You can do this by adding a new filter to a downloadable subscription, manually updating the subscription on the options page, and trying out the new filter. Turn off internet access to the device and try to sync the subscription again, then go online again, add another new filter, sync again, and see that the last added filter works.

Follow-up work

See #7393.

[sebastian]

Please add integration notes, to remove obsolete code from lib/compat.js in adblockpluschrome.

Also what would you think about using the fetch() API instead while at it?

[mjethani]

Replying to sebastian:

Please add integration notes, to remove obsolete code from lib/compat.js in adblockpluschrome.

Done.

Also what would you think about using the fetch() API instead while at it?

I like the idea, but that would require some study of the API to make sure it has the same semantics and fulfills the requirements. We should do it as a next step. In this step we get rid of the Cr namespace.

[abpbot]

A commit referencing this issue has landed:
​Issue 7376 - Remove references to XMLHttpRequest.channel

[mjethani]

@sebastian @kzar @Ross I need your help in updating the "Hints for testers" section to add something about the fallback URL. If download fails for 5 attempts (default setting), the extension will consult the fallback URL (which is https://adblockplus.org/getSubscription with some query string parameters by default), but I don't know if there's a way to test this or if tests already exist for this.

[mjethani]

Also I wonder if the server would have a problem since we are no longer going to be passing the channelStatus query string parameter to the fallback URL (see ​diff and integration notes).

[Ross]

I don't think any manual/QA tests exist for this, it's not actually even in the regression sheets. I've made a note to update them to include this. The tester could always just proxy the connection to check the fallback is actually requested (or even point the hostname at a local server).

It doesn't look like channelStatus is used by the server handler? (​https://hg.adblockplus.org/sitescripts/file/tip/sitescripts/subscriptions/web/fallback.py)

[mjethani]

Since apparently we are not testing the fallback URL feature, I have closed this without any hints for how to test the fallback URL path. I have filed #7393 to properly address that feature. As part of that, we can also write proper tests and make sure the feature is covered in our tests.

[Ross]

Done. Working as described.

ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2