647654 - [SMILE] Avoid binding this._event N times by doing it once in the constructor.

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Philip Chee]

From Bug 647571 Comment 2:
> [e-mail]      2011-04-03 13:15:37 PDT
> 
> [In theory it's possible to avoid binding this._event N times by doing it once
> in the constructor. Followup bug perhaps?]

Comment 1

[Serge Gautherie (:sgautherie)]

What is the syntax to do this?

Ftr, there are 2 occurrences:
{
143       this._cleanup[aType] = this._event.bind(this),
230       this._cleanup[aType] = this._event.bind(this),
}

Comment 2

[[e-mail]]

Something like

__event : function win_event(aEvent) {

...

this._event = this.__event.bind(this);

...

this._cleanup[aType] = this._event,

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) smileApplication.js: Bind '_event()' once for all [Checked in: See comment 7]

Should be like this.
(Untested.)

Comment 4

[[e-mail]]

(In reply to Serge Gautherie from comment #3)
> (Untested.)
Do we have unit tests that I can run locally for this?

Comment 5

[Serge Gautherie (:sgautherie)]

Yes: http://mxr.mozilla.org/comm-central/source/suite/smile/test/

Comment 6

[[e-mail]]

Comment on attachment 627134 [details] [diff] [review]
(Av1) smileApplication.js: Bind '_event()' once for all
[Checked in: See comment 7]

>+  // Define '_event()' as binded once for all.
Nit: past tense of 'bind' is 'bound', not 'binded'.

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 627134 [details] [diff] [review]
(Av1) smileApplication.js: Bind '_event()' once for all
[Checked in: See comment 7]

http://hg.mozilla.org/comm-central/rev/67501405df7b
Av1, with comment 6 suggestion(s).