Opened on 03/28/2017 at 08:01:06 AM

Last modified on 09/16/2019 at 03:44:27 PM

#5039 reviewing change

Add support of nullable values for non-objects in settings.

Reported by: sergz Assignee: sergz
Priority: P2 Milestone:
Module: Libadblockplus Keywords:
Cc: fhd, asmirnov, hfiguiere Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29396582/

Description (last modified by sergz)

Background

It turned out that current implementation does not support nullable values, e.g. for strings because the check of type does not pass. Let's say the expected type of variable is a string, in this case if there the default value is null it's not possible to set a string value because typeof null /*object*/ != typeof stringValue /*string*/ and vice versa (to set null).
One can argue that we can have a special value but it's a magic value and soon or late it will bite.

What to change

Adjust implementation in prefs.js to support optional values. An example where it works is a setting from #5040.
Also get rid if the hack introduced in #5040.

Attachments (0)

Change History (6)

comment:1 Changed on 03/28/2017 at 08:02:57 AM by sergz

  • Description modified (diff)

comment:2 Changed on 03/28/2017 at 09:21:53 AM by sergz

  • Blocking 5040 added
  • Description modified (diff)
  • Owner set to sergz
  • Priority changed from P3 to P1
  • Summary changed from Add support of nullable values in settings. to Add support of optional values in settings.

Basically, #5040 will be fixed with that issue.

comment:3 Changed on 03/28/2017 at 09:28:17 AM by sergz

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

comment:4 Changed on 03/29/2017 at 11:51:04 AM by sergz

  • Blocking 5040 removed

comment:5 Changed on 03/29/2017 at 11:51:30 AM by sergz

  • Priority changed from P1 to P2

comment:6 Changed on 03/29/2017 at 11:53:44 AM by sergz

  • Description modified (diff)
  • Summary changed from Add support of optional values in settings. to Add support of nullable values for non-objects in settings.

Add Comment

Modify Ticket

Change Properties
Action
as reviewing .
as The resolution will be set. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from sergz.
 
Note: See TracTickets for help on using tickets.