Closed Bug 791944 Opened 7 years ago Closed 7 years ago

Use Object.defineProperty() instead of __defineGetter__ in XPCOMUtils.defineLazyGetter()

Categories

(Core :: XPConnect, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: tetsuharu, Assigned: tetsuharu)

References

Details

Attachments

(1 file, 1 obsolete file)

Use Object.defineProperty() instead of __defineGetter__ in XPCOMUtils.defineLazyGetter()
Attached patch patch (obsolete) — Splinter Review
Attachment #662055 - Flags: review?(josh)
Comment on attachment 662055 [details] [diff] [review]
patch

Given that I know little about this file, I looked at the hg log, checked out the last bug to make a significant change and looked at the reviewer for that patch. Benjamin, you're up!
Attachment #662055 - Flags: review?(josh) → review?(benjamin)
Comment on attachment 662055 [details] [diff] [review]
patch

I'm going to delegate this to mak since .defineProperty isn't something I've worked with before, and he wrote a lot of this file.
Attachment #662055 - Flags: review?(benjamin) → review?(mak77)
Comment on attachment 662055 [details] [diff] [review]
patch

Review of attachment 662055 [details] [diff] [review]:
-----------------------------------------------------------------

Yes this works the same, __defineGetter__ is deprecated, so good to replace it.

Did you see bug 647423? Maybe you could be interested in filing dependencies of it and start doing a platform wide replacement.
Likely would also be a good first bug, fwiw.

::: js/xpconnect/loader/XPCOMUtils.jsm
@@ +176,5 @@
> +        delete aObject[aName];
> +        return aObject[aName] = aLambda.apply(aObject);
> +      },
> +      configurable: true,
> +      enumerable: true,

please remove the trailing comma from here
Attachment #662055 - Flags: review?(mak77) → review+
Blocks: 647423
already tested by current tests using defineLazy*Anything*
Assignee: nobody → saneyuki.s.snyk
Status: NEW → ASSIGNED
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Attached patch patch rev1.1Splinter Review
(In reply to Marco Bonardo [:mak] from comment #4)
> Did you see bug 647423? Maybe you could be interested in filing dependencies
> of it and start doing a platform wide replacement.
> Likely would also be a good first bug, fwiw.

Thank you, Marco. I'll look into it.
Attachment #662055 - Attachment is obsolete: true
Attachment #664384 - Flags: review?(mak77)
Attachment #664384 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/c89fff328c4c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.