Closed Bug 598983 Opened 14 years ago Closed 14 years ago

change "selection" module to use EventEmitter event registration model

Categories

(Add-on SDK Graveyard :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: irakli, Assigned: irakli)

References

()

Details

Attachments

(1 file)

      No description provided.
Assignee: nobody → rFobic
Comment on attachment 479081 [details] [diff] [review]
Implements changes

EventEmitter sets the |this| object in event handlers it calls to null.  We'll need to update it to set it to the object on which the event is defined.  But we can do that in another bug.
Attachment #479081 - Flags: review?(myk) → review+
If that has to be done it's definitely something for the other bug to tackle. I personally think that it's bad idea, since it's misleading for developers that are already familiar with an `EventEmitter` model and for casual developers it's even worse since switching to DOM will be confusing. 

let myObject = {
  listener: function(yourObject) {
    this == myObject // false
    this == yourObject // true
  }
}
yourObject.on('event', myObject.listener);

This snippet explains pretty well why IMO it's a bad idea. It also illustrates pretty well that listener has various ways to access yourObject. That being said I can live with this change since developers are able to write code in right way as well.
fixed: https://hg.mozilla.org/labs/jetpack-sdk/rev/9d0d0e065f22
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: