Opened on 01/20/2017 at 08:08:32 PM

Closed on 08/29/2019 at 05:49:52 PM

#4829 closed defect (rejected)

Empty error message when adding invalid filter

Reported by: greiner Assignee: sebastian
Priority: P2 Milestone:
Module: Platform Keywords: closed-in-favor-of-gitlab
Cc: kzar, sebastian, Ross, greiner Blocked By:
Blocking: #4275, #4275 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by kzar)


Adblock Plus
Chrome 55.0.2883.87
Ubuntu 16.04

How to reproduce

  1. Open the options page and navigate to "Add custom filters" tab.
  2. Attempt to add the filter ,##foo.

Observed behaviour

Empty modal dialog appears.

Expected behaviour

Modal dialog appears and contains further information about the error.


The filter validation that produces the error "filter_invalid_domain" was introduced in #4450 but without an error text associated with it. Therefore adding a translatable string with the ID "filter_invalid_domain" to adblockplus/chrome/locale/en-US/ should be sufficient to fix this issue.

The problem is the adblockplus repository is going away, we hardly want to add more strings there. So instead let's move the filter_* messages over into adblockplusui while we're at it.

What to change

  • Move the filter_* strings from in adblockplus to new-options.json in adblockplusui, synchronise the updated adblockplusui project with Crowdin. (No need to remove the strings from adblockplus nor update that project in Crowdin, since it's going away.)
  • Add the filter_invalid_domain string with the message of "Invalid (or empty) domain specified" .

Integration notes

You'll have to adjust adblockpluschrome/ when updating the adblockplusui dependency in order to take the filter_* strings from adblockplusui instead of adblockplus.

Hints for testers

Open the options page and trigger the various errors, make sure they are displayed properly:

  • filter_elemhide_duplicate_id
  • filter_elemhideemulation_nodomain
  • filter_elemhide_nocriteria
  • filter_invalid_domain
  • filter_invalid_regexp
  • filter_regexp_tooltip
  • filter_unknown_option

Attachments (0)

Change History (28)

comment:1 Changed on 01/21/2017 at 02:28:59 PM by mapx

  • Component changed from Adblock-Plus-for-Firefox to Platform

comment:2 Changed on 01/23/2017 at 03:28:54 AM by kzar

  • Cc sebastian trev added

Can reproduce as described, but this raises the question where these strings should live. Since we're working towards removing the adblockplus dependency from adblockpluschrome it seems wrong to add the string there.

Should we instead move all these strings over to adblockplusui and add filter_invalid_domain whilst at it? Should we instead start adding strings to adblockpluscore instead?

Either way I guess this issue will eventually change module, either become UI or core and we'll need further issues for the dependency updates.

comment:3 Changed on 01/23/2017 at 01:44:45 PM by trev

These strings definitely belong in adblockpluscore, this is where they are used. However, moving them from adblockplus to adblockpluscore isn't going to be entirely straightforward so we might want to add the missing string to adblockplus for now and address the migration independently.

comment:4 Changed on 01/23/2017 at 02:06:29 PM by kzar

OK, well I volunteer to open an issue for that and to do the work if you can create the adblockpluscore project for me on CrowdIn and give me access.

As for whether we should add the string to adblockplus first, I'd rather just fix this properly personally but I don't mind too much either way.

comment:5 Changed on 01/23/2017 at 02:28:36 PM by kzar

  • Blocking 4833 added

(I've opened #4833 for that, I'll set it as blocking for now but I don't mind if you undo.)

comment:6 Changed on 01/23/2017 at 02:29:10 PM by kzar

  • Blocked By 4833 added
  • Blocking 4833 removed


comment:7 Changed on 01/23/2017 at 03:30:41 PM by trev

  • Blocked By 4833 removed

comment:8 Changed on 01/23/2017 at 03:30:57 PM by trev

  • Blocked By 4833 added

comment:9 Changed on 01/23/2017 at 03:31:58 PM by trev

  • Blocked By 4833 removed

comment:10 Changed on 01/23/2017 at 03:33:10 PM by trev

  • Blocked By 4833 added

comment:11 Changed on 01/24/2017 at 11:32:04 AM by kzar

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

comment:12 Changed on 01/27/2017 at 07:34:16 AM by kzar

I prepared everything for these strings to go in adblockpluscore, but a discussion came up on IRC about weather they should really belong in adblockplusui.

Until that's decided I can't continue with this. Both sound reasonable to me so I don't mind if the strings end up in core or ui. Thomas & Wladimir what do you think?

For reference the strings in question are as follows, but note the text for filter_invalid_domain might still change:

  "filter_elemhide_duplicate_id": {
    "message": "Only one ID of the element to be hidden can be specified"
  "filter_elemhideemulation_nodomain": {
    "message": "No active domain specified for extended element hiding filter"
  "filter_elemhide_nocriteria": {
    "message": "No criteria specified to recognize the element to be hidden"
  "filter_invalid_domain": {
    "message": "Invalid (or empty) domain specified"
  "filter_invalid_regexp": {
    "message": "Invalid regular expression"
  "filter_regexp_tooltip": {
    "message": "This filter is either a regular expression or too short to be optimized. Too many of these filters might slow down your browsing."
  "filter_unknown_option": {
    "message": "Unknown filter option"

comment:13 Changed on 03/16/2017 at 09:11:11 AM by kzar

Sorry to nag you guys but should I put these strings in adblockplusui or adblockpluscore?

comment:14 Changed on 05/29/2017 at 03:39:58 PM by kzar

greiner, trev: So this is still blocked, any decision about if filter_invalid_domain and similar texts should live in adblockplusui or adblockpluscore?

comment:15 Changed on 08/16/2017 at 09:24:52 AM by trev

  • Blocked By 4833 removed

I just closed #4833, these strings should go in adblockplusui.

comment:16 Changed on 09/29/2017 at 08:52:21 AM by kzar

  • Blocking 5751 added
  • Cc greiner added
  • Component changed from Platform to User-Interface
  • Description modified (diff)
  • Platform changed from Chrome to Unknown / Cross platform
  • Status changed from reviewing to reopened

(Copying in Thomas since this is now an adblockplusui change.)

comment:17 Changed on 09/29/2017 at 08:54:28 AM by kzar

  • Cc greiner removed
  • Description modified (diff)

Whoops, just realised he filed the issue...

comment:18 Changed on 09/29/2017 at 08:55:46 AM by kzar

  • Description modified (diff)

comment:19 Changed on 09/29/2017 at 08:59:18 AM by kzar

  • Description modified (diff)

comment:20 Changed on 09/29/2017 at 09:00:33 AM by kzar

  • Blocking 4275 added; 5751 removed

comment:21 Changed on 09/29/2017 at 09:01:32 AM by kzar

  • Blocking

comment:22 Changed on 09/29/2017 at 07:42:38 PM by sebastian

If those strings aren't directly used in adblockplusui, they should rather go into adblockpluschrome, also see ticket:4275#comment:4. The reason we initially planned to move all shared strings to adblockplusui (regardless whether they are used there) was merely a suboptimal compromise, since they were also used by adblockplus which is deprecated now.

comment:23 Changed on 10/01/2017 at 01:13:56 AM by sebastian

  • Owner set to sebastian

comment:24 Changed on 10/01/2017 at 02:25:33 AM by sebastian

  • Component changed from User-Interface to Platform
  • Milestone set to Adblock-Plus-for-Chrome-Opera-next
  • Resolution set to fixed
  • Status changed from reopened to closed

comment:25 Changed on 11/02/2017 at 06:47:40 AM by Ross

Filter error messages are still blank/empty. (Firefox contains "Line 1")

Firefox 57 / Windows 10

Chrome 62 / Windows 10
Opera 48 / Windows 10

comment:26 Changed on 11/30/2017 at 07:49:26 PM by kzar

  • Cc Ross greiner added
  • Milestone Adblock-Plus-3.0-for-Chrome-Opera-Firefox deleted
  • Priority changed from Unknown to P2
  • Ready set
  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:27 Changed on 12/21/2017 at 11:31:47 AM by fhd

  • Cc trev removed

comment:28 Changed on 08/29/2019 at 05:49:52 PM by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from reopened to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

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 sebastian.
Note: See TracTickets for help on using tickets.