#5808 closed defect (fixed)

Easyprivacy URL is being shown instead of the title

Reported by: saroyanm Assignee: saroyanm
Priority: P2 Milestone:
Module: User-Interface Keywords:
Cc: greiner, wspee, sebastian Blocked By:
Blocking: #5158 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29559647/

Description

Environment

Chrome

How to reproduce

  1. Build and load Adblock Plus with the new options page bundled as described here
  2. Open settings page
  3. Turn on Disable Tracking
  4. Reload the extension
  5. Open options page and navidate to advanced tab
  6. Observe

Observed behaviour

In the Filter lists section Easyprivacy title is it's URL instead of the title

Expected behaviour

Easyprivacy title specified by the author is shown

Change History (10)

comment:1 Changed 22 months ago by saroyanm

This issue appears when Easyprivacy is being requested without title being specified, same is observable in current(old) option page.
If you request Fanboy without title, Adblock Plus will retrieve the filter list and set title, but if you will do same for Easyprivacy - No.

comment:2 Changed 22 months ago by saroyanm

  • Cc sebastian added

@Sebastian: Maybe you can help me here, do you know why when a Easyprivacy is being requested:

let subscription = Subscription.fromURL("https://easylist-downloads.adblockplus.org/easyprivacy+easylist.txt");
FilterStorage.addSubscription(subscription);

The subscription title appears to be subscription's url, but when with same logic another filter list is being requested (ex.: fanboy-social.txt) the title is being set to actual title of filter list specified by author.

This behavior is also possible to test with current stable Chrome extension, if the filer list is requested, but no title is specified.

comment:3 follow-up: Changed 22 months ago by sebastian

When I add https://easylist-downloads.adblockplus.org/easyprivacy+easylist.txt through the UI of the old/current options page, it first shows the URL (except I enter a title when adding the subscription), but once the filter list has been downloaded it shows "EasyPrivacy+EasyList" which is the Title given in the header of the filter list file.

This is the expected behavior, since the title is unknown (unless it was provided by the user or in a clicked abp:subscribe link), untill the filter list had been downloaded and the title has been parsed from its header.

If you add subscription with code, note that the snippet above won't trigget the filter list download. In order to do so, you have to explicitly call Synchronizer.execute().

comment:4 in reply to: ↑ 3 Changed 22 months ago by saroyanm

Replying to sebastian:

When I add https://easylist-downloads.adblockplus.org/easyprivacy+easylist.txt through the UI of the old/current options page, it first shows the URL (except I enter a title when adding the subscription), but once the filter list has been downloaded it shows "EasyPrivacy+EasyList" which is the Title given in the header of the filter list file.

You are right I missed another step in reproduction, aparently it happens when you add the list second time:

  1. Install extension from adblockplus.org
  2. Open option page
  3. Add https://easylist-downloads.adblockplus.org/easyprivacy+easylist.txt (without specifying title)
  4. Observe
  5. Remove Easyprivacy
  6. Repeat 3-rd step.
  7. Observe

On step 4: it updates the title as soon the list is downloaded.
On step 7: It's not and after that I can't see the title each time I ad the list, or restart the extension.

I'm not sure this might or might not be connected with the issue in this ticket, but still looks to me unexpected behavior, if I'm not missing something.

This is the expected behavior, since the title is unknown (unless it was provided by the user or in a clicked abp:subscribe link), untill the filter list had been downloaded and the title has been parsed from its header.

If you add subscription with code, note that the snippet above won't trigget the filter list download. In order to do so, you have to explicitly call Synchronizer.execute().

acknowledged, sorry for incomplete code.

Last edited 22 months ago by saroyanm (previous) (diff)

comment:5 follow-up: Changed 22 months ago by sebastian

Subscriptions are cached, so that even after the subscription has been removed, when added again, it is still the same object. So lastDownload is already set and this check in adblockplusui prevents it from being downloaded again.

Everything until this point works as intended, and if the same Subscription object, with all properties preserved, would just be added again, also the title should surely be correct (i.e. the same as before), and there should be no need to force the subscription to be downloaded again.

However, the bug in the old/current options page is that when adding a subscription without giving a title the URL is used as title, which is then causing the cached title to be overridden. I didn't debug the new options page, but chances are, even though the implementation is quite different, the same mistake has been made there again.

comment:6 Changed 22 months ago by saroyanm

  • Owner set to saroyanm

comment:7 Changed 22 months ago by saroyanm

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

comment:8 in reply to: ↑ 5 Changed 22 months ago by saroyanm

Replying to sebastian:

Subscriptions are cached, so that even after the subscription has been removed, when added again, it is still the same object. So lastDownload is already set and this check in adblockplusui prevents it from being downloaded again.

Everything until this point works as intended, and if the same Subscription object, with all properties preserved, would just be added again, also the title should surely be correct (i.e. the same as before), and there should be no need to force the subscription to be downloaded again.

Acknowledged

However, the bug in the old/current options page is that when adding a subscription without giving a title the URL is used as title, which is then causing the cached title to be overridden. I didn't debug the new options page, but chances are, even though the implementation is quite different, the same mistake has been made there again.

In the new options page we didn't introduce same mistake. Seems like I identified the problem, that was caused by inconsistent way of handling how subscriptions were added, but can't say for sure what exactly were causing assignment of URL to title.

comment:9 Changed 22 months ago by abpbot

A commit referencing this issue has landed:
Issue 5808 - Easyprivacy URL is being shown instead of the title

comment:10 Changed 22 months ago by saroyanm

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.