Opened 10 months ago

Closed 10 months ago

Last modified 9 months ago

#6417 closed defect (fixed)

empty error message for incorrect custom filters

Reported by: mapx Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-3.0.3-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: kzar, mjethani, greiner Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29709602/

Description (last modified by kzar)

Environment

ABP last dev build (FF / chrome) 3.0.2.1975
FF beta / chrome beta
windows 7 / windows 10

Ubuntu 17.10
Chrome 64
Adblock Plus 3.0.2.1975

How to reproduce

Add the custom filter @@||example.com^$first-party or ,###whatever

Observed behaviour

Modal dialog opens displaying only the row number of the incorrect filter, no error message.

Further information

We use Utils.getString() in lib/filterValidation.js to retrieve the error message for the given error reason. However, unlike browser.i18n.getMessage() it returns an empty string instead.

Attachments (1)

errorabp.PNG (12.3 KB) - added by mapx 10 months ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 months ago by mapx

  • Cc greiner added

comment:2 Changed 10 months ago by mapx

  • Component changed from Platform to User-Interface

comment:3 Changed 10 months ago by greiner

I wasn't able to reproduce this by adding the filter [ in the custom filter list. The filter is simply not added and no error message is shown (that's because #5549 is not included in this release yet).

Ubuntu 17.10
Chrome 64
Adblock Plus 3.0.2.1975

Which filter did you use to trigger that message?

Changed 10 months ago by mapx

comment:4 Changed 10 months ago by mapx

@@||example.com^$first-party

which obviously is not accepted by ABP (windows 7 / windows 10)

see attachment

Last edited 10 months ago by mapx (previous) (diff)

comment:5 Changed 10 months ago by mapx

  • Description modified (diff)

comment:6 follow-up: Changed 10 months ago by greiner

  • Component changed from User-Interface to Platform
  • Description modified (diff)

Thanks. I was able to reproduce it with this filter and identify what's causing this issue.

comment:7 Changed 10 months ago by kzar

  • Description modified (diff)
  • Owner set to kzar
  • Priority changed from Unknown to P3
  • Ready set

comment:8 Changed 10 months ago by kzar

  • Priority changed from P3 to P2

comment:9 in reply to: ↑ 6 Changed 10 months ago by kzar

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

I don't understand why Utils.getString() was there at all, as far as I can see it's only being used in this one place and as we found out it didn't work there.

comment:10 Changed 10 months ago by abpbot

comment:11 Changed 10 months ago by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:12 Changed 9 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Fixed. Line number and error messages are now displayed as expected.

Firefox 53 / Firefox 58 / Windows 7
Chrome 49 / 65 / Windows 7
Opera 36 / 51 / Windows 7

Note: See TracTickets for help on using tickets.