Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#48 closed defect (fixed)

Don't show Homepage link in filter preferences if filterlist doesn't have a homepage associated with it

Reported by: greiner Assignee: trev
Priority: P3 Milestone: Adblock-Plus-2.6-for-Firefox
Module: Adblock-Plus-for-Firefox Keywords:
Cc: trev, fhd, greiner Blocked By:
Blocking: Platform:
Ready: no Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5398934247702528/

Description (last modified by trev)

Adblock Plus for Firefox will show a Homepage link even for the filter lists that don't have any known homepage.

How to reproduce

  1. Click Adblock Plus icon and select Filter Preferences from the menu.
  2. Click "Add filter subscription" button.
  3. Click "Add a different subscription" link.
  4. Enter "foo" as subscription title and "http://bar" as filter list location, confirm.
  5. Move over the Homepage link for the "foo" subscription.

Observed behaviour

The tooltip for the "Homepage" link says "null" and that URL is actually opened if it is clicked.

Expected behaviour

As there is no correct website to link to for those lists, there should be no link for that at all.

Change History (15)

comment:1 Changed 6 years ago by philll

  • Description modified (diff)

comment:2 Changed 6 years ago by trev

  • Cc trev fhd added

I am about to resolve this as "by design". We are talking about recommended filter lists here, having a dedicated homepage that the users can turn to is one of the requirements for being recommended. If anything, we should change the script generating the list of recommendations to warn about filter lists without a homepage.

Felix, do you agree?

Last edited 6 years ago by trev (previous) (diff)

comment:3 follow-up: Changed 6 years ago by trev

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

I just realized that this isn't merely about recommendations but about the full list as well. However, having at least some URL for a filter list is already being enforced. sitescripts.subscriptions.subscriptionParser module will reject filter lists that have none of the homepage, forum, blog, faq and contact fields. The first of these fields that is present is taken over for the homepage attribute of the subscription.

Resolving as worksforme.

comment:4 Changed 6 years ago by philll

  • Cc greiner added
  • Description modified (diff)

Greiner, could you please verify this?

comment:5 in reply to: ↑ 3 Changed 6 years ago by greiner

Replying to trev:

I just realized that this isn't merely about recommendations but about the full list as well. However, having at least some URL for a filter list is already being enforced. sitescripts.subscriptions.subscriptionParser module will reject filter lists that have none of the homepage, forum, blog, faq and contact fields. The first of these fields that is present is taken over for the homepage attribute of the subscription.

Resolving as worksforme.

This only applies to filterlists that are hosted on our servers. This issue, however, is about any list that can be added to the extension.

comment:6 Changed 6 years ago by trev

  • Component changed from Unknown to ABP-Firefox
  • Description modified (diff)
  • Priority changed from Unknown to P3
  • Resolution worksforme deleted
  • Status changed from closed to reopened

So much for not having useful steps to reproduce. I thought this was about the "Add filter subscription" button.

Reopening and changing description, this is about Adblock Plus for Firefox and the subscription listing.

comment:7 Changed 6 years ago by trev

Looking at the subscription template in chrome/content/ui/filters.xul, displaying the Homepage link is already conditional. It's not immediately obvious why it is still being displayed despite of that, I am pretty sure that this is a regression.

comment:8 Changed 6 years ago by trev

  • Owner set to trev
  • Status changed from reopened to assigned

comment:10 Changed 6 years ago by philll

  • Status changed from assigned to reviewing

comment:11 Changed 6 years ago by trev

Heh, mistakenly pushed the change already: https://hg.adblockplus.org/adblockplus/rev/1f0936096072

comment:12 Changed 6 years ago by trev

  • Reporter changed from philll to greiner

comment:13 Changed 6 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:14 Changed 6 years ago by trev

  • Milestone set to Adblock-Plus-for-Firefox-next

comment:15 Changed 6 years ago by trev

  • in_progress set to 0
  • Ready unset
  • Review URL(s) modified (diff)
Note: See TracTickets for help on using tickets.