Closed Bug 452381 Opened 12 years ago Closed 12 years ago

Don't pass strings to setTimeout

Categories

(Calendar :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: martinschroeder, Assigned: martinschroeder)

Details

Attachments

(1 file)

Clone of bug 431978 for Calendar:
As pretty much every guide to either efficient or secure JavaScript says,
setTimeout("foopy()", 0) is almost never what you want to do.
Attached patch Patch v1Splinter Review
Attachment #341637 - Flags: review?(philipp)
Comment on attachment 341637 [details] [diff] [review]
Patch v1

>-        setTimeout("unifinderTreeView.resetAllowSelection()", 1);
>+        setTimeout(unifinderTreeView.resetAllowSelection, 1);
I believe calling setTimeout() like this will result in resetAllowSelection being called without unifinderTreeView being the |this| context. Please check.


r=philipp with that checked.
Attachment #341637 - Flags: review?(philipp) → review+
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/bf0a9456e137>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
-> REOPENING
>-        setTimeout("unifinderTreeView.resetAllowSelection()", 1);
>+        setTimeout(unifinderTreeView.resetAllowSelection, 1);
this change broke selection of events: unifinder -> calendar view
reverting this part... ?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #5)
> -> REOPENING
> >-        setTimeout("unifinderTreeView.resetAllowSelection()", 1);
> >+        setTimeout(unifinderTreeView.resetAllowSelection, 1);
> this change broke selection of events: unifinder -> calendar view
> reverting this part... ?

Please, provide steps to reproduce this issue. I tested this part of the code before checkin (also code-wise), and everything seemed to be fine.
well:
1. select event in unifinder
2. event is not selected in the calendar view (day, week etc.) - nothing changes

Confirming on latest build 20081025043208 (TB + Lightning) Win+Mac

Reverting this part fixes it for me.
Regression will be fixed in bug 461944.
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.