Closed
Bug 593737
Opened 13 years ago
Closed 13 years ago
change event-generating high-level modules to use EventEmitter event registration model
Categories
(Add-on SDK Graveyard :: General, defect, P1)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
0.8
People
(Reporter: myk, Assigned: irakli)
References
Details
Attachments
(1 file, 1 obsolete file)
12.57 KB,
patch
|
Details | Diff | Splinter Review |
Bug 588732 implements the EventEmitter model for event registration. There should be one good way to register events in the SDK, so all event-generating high-level modules should be changed to use the EventEmitter model. The implementation in bug 588732 relies on Traits, and it's unclear how usable it is with modules that use lexical scoping to hide private properties. Nevertheless, I highly recommend that whoever takes responsibility for this bug implement the minimal changes necessary to switch modules to the EventEmitter model. In other words, if there's a way to make the changes that doesn't require switching all those modules from lexical scope implementations to Traits implementations, we should use that way. Even if we eventually change to using Traits everywhere, now is not the time to be doing so, as we have more than enough work to do already to get 0.8 released as our first beta. Irakli: do you have a sense of what minimal changes are required to switch existing high-level modules using lexical scoping to the EventEmitter model?
Assignee | ||
Comment 1•13 years ago
|
||
gist containing diff with changes: http://gist.github.com/566773 While changes here are pretty trivial here there are cases like 'tabs' where it will be more complicated, but that's a different issue, caused by the fact that code calling listeners is relying on a fact that listeners can be enumerated outside of the constructors lexical scope, which I believe had to be addressed by bug 588250 anyway. Please also keep in mind that code is just for demo and it was not tested, it may be missing.
Assignee | ||
Comment 2•13 years ago
|
||
Diff with master: http://github.com/Gozala/jetpack-sdk/compare/master...events@588732 Commit: http://github.com/Gozala/jetpack-sdk/commit/8b6c48a0bffea75eb661f8a97b4b8c6ec2a33dd9
Attachment #472340 -
Attachment is obsolete: true
Attachment #472693 -
Flags: review?(myk)
Assignee | ||
Comment 3•13 years ago
|
||
please ignore my last comment I meant to attach patch to the different bug.
Assignee | ||
Updated•13 years ago
|
Attachment #472693 -
Flags: review?(myk)
Reporter | ||
Comment 4•13 years ago
|
||
Irakli: is this something you could take on after finishing the ContentSymbiont work? It's necessary in order for us to declare a beta, which makes it a high priority (more important than Sidebar). Note: the key here is to make the minimal changes required to get high-level modules using EventEmitter-style event listener registration. This is not the time to be rewriting all those modules to work as Traits.
Priority: P2 → P1
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → rFobic
Assignee | ||
Comment 5•13 years ago
|
||
All my patches are submitted for review so I can work on this before I'll get feedback on ContentSymbiont or PageMod. Can you plese list all the modules that will need to change.
Assignee | ||
Updated•13 years ago
|
Reporter | ||
Comment 6•13 years ago
|
||
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
Reporter | ||
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•