Opened on 06/06/2014 at 10:49:00 PM
Closed on 07/10/2014 at 05:05:06 PM
Last modified on 05/20/2015 at 02:22:39 PM
#653 closed change (fixed)
Object.defineProperty instead of defineGetter / defineSetter
| Reported by: | tschuster | Assignee: | tschuster |
|---|---|---|---|
| Priority: | Unknown | Milestone: | Adblock-Plus-2.6.4-for-Firefox |
| Module: | Adblock-Plus-for-Firefox | Keywords: | |
| Cc: | trev | Blocked By: | |
| Blocking: | #312 | Platform: | Firefox |
| Ready: | yes | Confidential: | no |
| Tester: | Verified working: | no | |
| Review URL(s): | |||
Description (last modified by tschuster)
We should use the standard Object.defineProperty function. It also allows us to override getters with plain value property, which is something we often want for some of the "lazy caching" getters that we use.
Attachments (0)
Change History (11)
comment:2 Changed on 06/07/2014 at 04:00:59 PM by philll
comment:3 Changed on 06/09/2014 at 09:32:33 AM by tschuster
comment:4 Changed on 06/09/2014 at 09:35:33 AM by tschuster
- Blocking 312 added
comment:5 Changed on 07/09/2014 at 05:17:12 PM by tschuster
- Platform set to Unknown
I just committed this: https://hg.adblockplus.org/adblockplus/rev/69871d10cf2a.
Testing:
I am sorry to say that this is patch doesn't have any specific target area, so you should make sure Adblockplus still works probably after this patch.
comment:6 Changed on 07/09/2014 at 05:17:36 PM by tschuster
- Resolution set to fixed
- Status changed from new to closed
comment:7 Changed on 07/09/2014 at 07:14:52 PM by tschuster
- Resolution fixed deleted
- Status changed from closed to reopened
Backout:
https://hg.adblockplus.org/adblockplus/rev/17869a1b0f95
We actually try to modify FilterStorage.sourceFile in our tests: https://hg.adblockplus.org/adblockplustests/file/a22b38cb9212/chrome/content/common.js#l115
Easiest solution is to use {configurable: true} when defining it.
comment:9 Changed on 07/10/2014 at 05:05:06 PM by tschuster
- Resolution set to fixed
- Status changed from reopened to closed
Relanded with configurable sourceFile:
https://hg.adblockplus.org/adblockplus/rev/309d8a2b3949
Fixed test suite to treat it as a normal property:
https://hg.adblockplus.org/adblockplustests/rev/1ef3d5a601d4
comment:10 Changed on 07/15/2014 at 05:05:32 PM by trev
- Component changed from Unknown to Adblock-Plus-for-Firefox
- Milestone set to Adblock-Plus-for-Firefox-next
- Platform changed from Unknown to Firefox/Firefox Mobile
comment:11 Changed on 05/20/2015 at 02:22:39 PM by philll
- Platform changed from Firefox/Firefox Mobile to Firefox
Made Firefox and Firefox mobile available as seperate platforms.
I trained SpamBayed to let this through now.