616907 - Convert private-browsing events to style described in bug 593921

Status: RESOLVED FIXED

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

Description

[Drew Willcoxon :adw]

Attachment: patch

Bug 593921 comment 8 says private-browsing's event listeners shouldn't be passed anything, but to be consistent with other modules that emit events they should be passed event objects whose `emitter` properties the modules themselves.

Also, private-browsing's events currently leak a private object via `this`, so I've fixed that.  `this` is now exports, i.e., the private-browsing module.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 495427 [details] [diff] [review]
patch

(In reply to comment #0)
> Bug 593921 comment 8 says private-browsing's event listeners shouldn't be
> passed anything, but to be consistent with other modules that emit events they
> should be passed event objects whose `emitter` properties the modules
> themselves.

That comment only says that because the current documentation doesn't mention any parameters for these event handlers, and the comment is trying to identify the mimimal amount of work that would resolve the beta blocker.  Since adding an event parameter later wouldn't be a breaking change, the comment doesn't address doing so.

Nevertheless, you're right that these handlers, like all event handlers in high-level modules, should be passed event objects with `emitter` properties.


> Also, private-browsing's events currently leak a private object via `this`, so
> I've fixed that.  `this` is now exports, i.e., the private-browsing module.

Awesome!

Comment 2

[Drew Willcoxon :adw]

https://github.com/mozilla/addon-sdk/commit/9d923564d414e56074d20f01f4e303fdcf48fd35

Comment 3

[Myk Melez [:myk] [@mykmelez]]

https://github.com/mozilla/addon-sdk/commit/1fa2e7900f18dbaebc98459f83ac2b252a35385a