Opened 8 weeks ago

Closed 7 weeks ago

Last modified 5 weeks 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 7 weeks ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 weeks ago by mapx

  • Cc greiner added

comment:2 Changed 7 weeks ago by mapx

  • Component changed from Platform to User-Interface

comment:3 Changed 7 weeks 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 7 weeks ago by mapx

comment:4 Changed 7 weeks ago by mapx

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

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

see attachment

Last edited 7 weeks ago by mapx (previous) (diff)

comment:5 Changed 7 weeks ago by mapx

  • Description modified (diff)

comment:6 follow-up: Changed 7 weeks 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 7 weeks ago by kzar

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

comment:8 Changed 7 weeks ago by kzar

  • Priority changed from P3 to P2

comment:9 in reply to: ↑ 6 Changed 7 weeks 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 7 weeks ago by abpbot

comment:11 Changed 7 weeks 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 5 weeks 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.