Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#2664 closed change (fixed)

Remove ignoreHeaders argument from parseFilter(s) and move the logic to the UI

Reported by: sebastian Assignee: sebastian
Priority: P4 Milestone: Adblock-Plus-1.9-for-Chrome-Opera-Safari
Module: Platform Keywords:
Cc: Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/4515985834901504

Description (last modified by sebastian)

Background

After #2658, trev suggested - and I agree - to also get rid of the ignoreHeaders argument in favor of slightly more complex data structures, leaving the handling of that case to the UI code.

What to change

  • Replace the error string by an error object, with a toString() method, and a type property in order to distinguish unexpected filter list headers from other reasons a filter is considered invalid.
  • Remove the ignoreHeaders argument from parseFilter() and parseFilters().
  • Make parseFilters() return an array of errors like {filters: [..], errors: [..]} instead of bailing out on the first validation error.
  • Adjust UI code and tests.

Hints for testers

Make sure that filter validation still works as it did before. See 2658#Hintsfortesters for detailed instructions.

Also pay particular attention to the handling of filter list headers. For example, if you enter [Adblock Plus 2.0] in raw edit mode on the options page, that line should simply be stripped. However, if you attempt to add that string using the single-line text field at the top, you should get an error message.

Change History (4)

comment:1 Changed 4 years ago by sebastian

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

comment:2 Changed 4 years ago by sebastian

  • Description modified (diff)

comment:3 Changed 4 years ago by sebastian

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

comment:4 Changed 4 years ago by sebastian

  • Description modified (diff)
Note: See TracTickets for help on using tickets.