Opened 4 years ago

Closed 4 years ago

#11699 closed defect (fixed)

wxTimer doesn't allow multiple timers with the same ID (MSW)

Reported by: aldimond Owned by: robind
Priority: blocker Milestone:
Component: wxMSW Version:
Keywords: Cc:
Blocked By: Blocking:
Patch: yes

Description

This began with r60296 (r60297 in 2.8 branch). This prevents constant IDs from being used in a class' event table for wxTimer, as is traditional.

I'll attach a patch I've made against the 2.8 branch for now, and I'll work on one for trunk.

Attachments (5)

awd_timer_id_2.patch download (3.5 KB) - added by aldimond 4 years ago.
Patch against 2.8 branch
awd_timer_id_28_a.patch download (3.1 KB) - added by aldimond 4 years ago.
Update of the patch against 2.8 branch
awd_timer_id_trunk_a.patch download (3.8 KB) - added by aldimond 4 years ago.
Patch against trunk
awd_timer_isrunning_28.patch download (875 bytes) - added by aldimond 4 years ago.
Fix for IsRunning() bug against 2.8 branch
awd_timer_isrunning_trunk.patch download (1.6 KB) - added by aldimond 4 years ago.
Fix for IsRunning() bug against trunk

Download all attachments as: .zip

Change History (17)

Changed 4 years ago by aldimond

Patch against 2.8 branch

comment:1 Changed 4 years ago by robind

Thanks for your efforts on this issue.

The change in timer.h should not be done in the 2.8 version to preserve binary compatibility.

MSDN states that the return value of SetTimer is zero when it fails, so I think the "ret != m_id" test is not correct. http://msdn.microsoft.com/en-us/library/ms644906(VS.85).aspx

comment:2 Changed 4 years ago by aldimond

Ah, I guess I misread the MSDN -- I thought it always returned the ID it was using, but that's not specified for the case where hWnd is set. Good spot.

Makes sense not to change timer.h also.

I'll upload some new patches in a minute. The trunk one is not tested very well because I am really bad at Windows stuff and Audacity doesn't build against trunk.

Changed 4 years ago by aldimond

Update of the patch against 2.8 branch

Changed 4 years ago by aldimond

Patch against trunk

comment:3 Changed 4 years ago by robind

  • Owner set to robind
  • Status changed from new to accepted

comment:4 Changed 4 years ago by RD

(In [63420]) Allow more than one timer with the same ID. See #11699.

comment:5 Changed 4 years ago by RD

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [63422]) Allow more than one timer with the same ID. Closes #11699.

comment:6 Changed 4 years ago by robind

Thanks for the patches!

comment:7 Changed 4 years ago by botg

  • Priority changed from normal to blocker
  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:8 Changed 4 years ago by botg

Unfortunately this patch has introduced a regression. wxTimer::IsRunning now always returns true.

Changed 4 years ago by aldimond

Fix for IsRunning() bug against 2.8 branch

Changed 4 years ago by aldimond

Fix for IsRunning() bug against trunk

comment:9 Changed 4 years ago by aldimond

Sorry about that, I didn't notices that m_id was used for IsRunning(). Newly-attached patches should fix the problem.

comment:10 Changed 4 years ago by RD

(In [63485]) Apply new patch fixing problem of IsRunning always returning True. See #11699

comment:11 Changed 4 years ago by RD

(In [63486]) Apply new patch fixing problem of IsRunning always returning True. See #11699

comment:12 Changed 4 years ago by robind

  • Resolution set to fixed
  • Status changed from reopened to closed

Thanks for the updates.

Note: See TracTickets for help on using tickets.