Don't pass strings to setTimeout

RESOLVED FIXED in 1.0b1

Status

RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: martinschroeder, Assigned: martinschroeder)

Tracking

unspecified
1.0b1

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
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.
(Assignee)

Comment 2

10 years ago
Created attachment 341637 [details] [diff] [review]
Patch v1
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+
(Assignee)

Comment 4

10 years ago
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/bf0a9456e137>

-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0

Comment 5

10 years ago
-> 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 → ---
(Assignee)

Comment 6

10 years ago
(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.

Comment 7

10 years ago
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.
(Assignee)

Comment 8

10 years ago
Regression will be fixed in bug 461944.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 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.