Opened on 03/16/2019 at 09:58:35 PM

Closed on 03/22/2019 at 09:34:43 AM

Last modified on 07/25/2019 at 07:53:40 PM

#7376 closed change (fixed)

Remove references to

Reported by: mjethani Assignee: mjethani
Priority: P3 Milestone:
Module: Core Keywords:
Cc: sebastian, hfiguiere, kzar, Ross Blocked By:
Blocking: #5702 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

Description (last modified by mjethani)


adblockpluscore still uses internal Gecko APIs (#5702) via wrappers. In practice they do nothing. One of these APIs is 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 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.

Attachments (0)

Change History (18)

comment:1 Changed on 03/16/2019 at 10:04:16 PM by mjethani

  • Description modified (diff)

comment:2 Changed on 03/16/2019 at 10:05:19 PM by mjethani

  • Description modified (diff)

comment:3 Changed on 03/16/2019 at 10:05:30 PM by mjethani

  • Owner set to mjethani

comment:4 Changed on 03/16/2019 at 10:10:21 PM by mjethani

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:5 Changed on 03/16/2019 at 10:11:24 PM by mjethani

  • Blocking 5702 added

comment:6 Changed on 03/16/2019 at 10:13:18 PM by mjethani

  • Cc sebastian hfiguiere added
  • Ready set

comment:7 follow-up: Changed on 03/17/2019 at 03:20:03 AM by 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?

comment:8 Changed on 03/17/2019 at 06:31:21 AM by mjethani

  • Description modified (diff)

comment:9 in reply to: ↑ 7 Changed on 03/17/2019 at 06:32:51 AM by mjethani

Replying to 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?

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.

comment:10 Changed on 03/18/2019 at 06:54:17 PM by abpbot

A commit referencing this issue has landed:
Issue 7376 - Remove references to

comment:11 Changed on 03/19/2019 at 10:10:51 AM by mjethani

  • Description modified (diff)

comment:12 Changed on 03/19/2019 at 10:11:07 AM by mjethani

  • Cc kzar Ross added

comment:13 Changed on 03/19/2019 at 10:13:37 AM by 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 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.

comment:14 Changed on 03/19/2019 at 10:15:11 AM by 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).

comment:15 Changed on 03/19/2019 at 11:03:25 AM by 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? (

comment:16 Changed on 03/22/2019 at 09:34:43 AM by mjethani

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:17 Changed on 03/22/2019 at 09:36:03 AM by 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.

comment:18 Changed on 07/25/2019 at 07:53:40 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Working as described.

Microsoft Edge 44.17763.1.0 / Windows 10 1809

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

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from mjethani.
Note: See TracTickets for help on using tickets.