Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#1504 closed defect (worksforme)

When the filter subscription is deleted and added, "Failed, download failure" is displayed

Reported by: passbrains Assignee:
Priority: Unknown Milestone:
Module: Unknown Keywords:
Cc: sebastian, mapx, trev, Shikitita Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

Description (last modified by sebastian)

Adapted from https://www.passbrains.com/dashboard/view-ticket.php?ticket_no=ACH-83

Environment

Windows + 8 64bit + Chrome + English
ABP version 1.8.6.1239

How to reproduce

  1. Add the 'ABP' plugin to the Chrome browser.
  1. Click on 'ABP" at top right hand corner.
  2. Click over the 'Options' --> Tap 'Add filter subscription ' tab.
  3. Select any of the filter subscription and delete (tap on 'x') before the download completes.
  1. Tap 'Add filter subscription' tab and add the same filter subscription in

the above step.

  1. Observe that "Failed, download failure" is displayed how many ever

times  filter subscription is deleted and added.

Observed behaviour

"Failed, download failure" is displayed how many ever
times filter subscription is deleted and added.

Expected behaviour

When a Filter subscription is added, it is downloaded.

Attachments (1)

3766_1414499292_download.mp4 (1.2 MB) - added by passbrains 5 years ago.

Download all attachments as: .zip

Change History (13)

Changed 5 years ago by passbrains

comment:1 Changed 5 years ago by sebastian

  • Cc sebastian added
  • Component changed from Unknown to Platform
  • Description modified (diff)
  • Priority changed from Unknown to P4
  • Ready set

comment:2 Changed 5 years ago by mapx

  • Cc mapx added

comment:4 Changed 4 years ago by sebastian

  • Cc trev added
  • Component changed from Platform to Core
  • Description modified (diff)
  • Platform changed from Chrome to Unknown

According to the duplicate linked above, this issue is also reproducible on Firefox.

Moving this issue to "Core" as that is where filter list downloads are handled.

comment:5 Changed 4 years ago by sebastian

  • Priority changed from P4 to Unknown

Also setting the priority to Unknown, leaving it up to the "Core" module owner. Initially I set it to P4, mistakenly assuming that it only effects the old UI on Chrome. But this doesn't seem to be the case anymore.

comment:6 Changed 4 years ago by trev

  • Cc Shikitita added
  • Component changed from Core to Unknown

#2696 isn't a duplicate - it's about the fact that subscription state persists even if a subscription is removed and added again, this is by design. Subscription download failing if the subscription is removed while downloading isn't the same thing.

@Shikitita, sebastian: Was either of you able to reproduce this issue? I wasn't. I have a strong suspicion that the OP's network connection simply wasn't switched on. Note that EasyList Hebrew and ROList aren't actually being downloaded in the video - these subscriptions were added before, downloaded successfully, and were merely re-added now. The download of EasyList Lithuania probably simply timed out while the subscription was removed (no network connection). So the error message when the subscription was re-added reflects the state correctly.

So I am inclined to resolve this as worksforme.

comment:7 Changed 4 years ago by sebastian

  • Description modified (diff)

This issue is quite old. So I don't remember what I reproduced back then. But I cannot reproduce the issue anymore. However, it seems that removing a subscription doesn't have any effect on its downloading state. So if you remove a subscription and add it again, it isn't downloaded again. Also if I remove a subscription while downloading and add it again, it's still downloading or finished while it was removed. So I assume that for OP, the download failed for some reason while it was removed, showing the error message when he added it again, which wouldn't be a bug.

But doesn't that mean that removing a subscription (by clicking the X), won't do anything but disabling it?

comment:8 Changed 4 years ago by trev

  • Resolution set to worksforme
  • Status changed from new to closed

But doesn't that mean that removing a subscription (by clicking the X), won't do anything but disabling it?

Yes, more or less - the Subscription object isn't destroyed, merely removed from the list.

comment:9 follow-up: Changed 4 years ago by Shikitita

I didn't manage to reproduce it either.

Having been able to stop two downloads successfully right during the downloading process, it didn't trigger the "Failed, download failure" status after adding them again, but were displayed as "Last updated at ... today". Therefore I agree and it is not an issue, then.

I just have a question now regarding the Subscription.

If the Subscription is not destroyed but just removed from the list, does that mean, technically, that the user is still subscribed to that filter? And if so, would that mean that the filter is loaded while ABP is running or only when ABP is loaded for the first time, say beginning of the day?

comment:10 in reply to: ↑ 9 Changed 4 years ago by trev

Replying to Shikitita:

If the Subscription is not destroyed but just removed from the list, does that mean, technically, that the user is still subscribed to that filter?

No. The filter list still exists in memory and will be reused if added again. However, we will no longer trigger updates for it, and it will be gone forever if Adblock Plus is restarted.

comment:11 Changed 4 years ago by sebastian

This sounds like a memory leak, that we should probably fix. Or is there any point in keeping these subscriptions around? From an UX aspect it seems to be rather confusing. And technically I don't see why caching removed subscriptions would make sense either.

comment:12 Changed 4 years ago by trev

Sure, caching is by definition a memory leak. However, it is hard to create enough Subscription objects for that to matter. It makes sense because people will sometimes remove a subscription yet reconsider afterwards.

Note: See TracTickets for help on using tickets.