Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#1434 closed defect (fixed)

Remove remaining non-standard JS usage from buildtools

Reported by: tschuster Assignee: sebastian
Priority: P4 Milestone:
Module: Automation Keywords:
Cc: sebastian, trev Blocked By: #3260, #3418, #3421
Blocking: #312 Platform: Unknown
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5462707926990848

Description (last modified by sebastian)

Background

We already removed non-standard JavaScript code from the adblockplus repository, from prefs.js (#3418), and partly from keySelector.js (#3421) and bootstrap.js.tmpl (#3260). But there is still some non-standard code left in the buildtools repository which is bundled with Firefox extensions.

What to change

  • Replace {__proto__: null} with Object.create(null) in bootstrap.js.tmpl and lib/keySelector.js.
  • Replace __defineGetter__() and __defineSetter__() with Object.defineProperty() in lib/hooks.js.
  • Replace __lookupGetter__() and __lookupSetter__() with Object.getOwnPropertyDescriptor() in lib/hooks.js.

Change History (20)

comment:1 Changed 5 years ago by tschuster

  • Review URL(s) modified (diff)

comment:2 Changed 4 years ago by sebastian

  • Cc sebastian trev added
  • Component changed from Unknown to Build-and-Release-Tools

I'm not sure whether we want to cleanup jshydra. As far as I know this code was forked, and doing changes like that, rebasing in the future gets just more difficult. Also jshydra depends on SpiderMonkey anyway. However, we should get rid of the non-standard code in the buildtools repository.

comment:3 Changed 4 years ago by trev

Yes, we definitely don't want to change JSHydra, and even changing our own code there isn't worth the effort. So this sounds like a duplicate of #2013.

comment:4 Changed 4 years ago by sebastian

  • Description modified (diff)
  • Ready set

I would prefer to close #2013 as duplicate of this issue, since it was filed later and only addresses a subset of these changes.

Last edited 4 years ago by sebastian (previous) (diff)

comment:5 Changed 4 years ago by sebastian

  • Priority changed from Unknown to P4

comment:6 Changed 4 years ago by sebastian

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

comment:7 Changed 4 years ago by sebastian

  • Owner set to sebastian

comment:8 Changed 4 years ago by sebastian

  • Description modified (diff)

comment:9 Changed 4 years ago by sebastian

  • Summary changed from Remove some non-standard JS usage from buildtools to Remove non-standard JS usage from buildtools

comment:10 Changed 4 years ago by sebastian

  • Description modified (diff)

comment:11 Changed 4 years ago by sebastian

  • Description modified (diff)

comment:12 Changed 4 years ago by sebastian

  • Description modified (diff)

comment:13 Changed 4 years ago by sebastian

  • Blocked By 3418 added

comment:14 Changed 4 years ago by sebastian

  • Description modified (diff)
  • Summary changed from Remove non-standard JS usage from buildtools to Remove remaining non-standard JS usage from buildtools
  • Tester set to Unknown

comment:15 Changed 4 years ago by sebastian

  • Description modified (diff)

comment:16 Changed 4 years ago by sebastian

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:17 Changed 4 years ago by sebastian

  • Blocked By 3421 added
  • Description modified (diff)

comment:18 Changed 4 years ago by sebastian

  • Blocked By 3260 added

comment:19 Changed 4 years ago by sebastian

  • Description modified (diff)

comment:20 Changed 4 years ago by sebastian

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