Closed
Bug 423395
Opened 16 years ago
Closed 16 years ago
onEventSweepMouseMove handler stays around after drop
Categories
(Calendar :: Calendar Frontend, defect)
Calendar
Calendar Frontend
Tracking
(Not tracked)
RESOLVED
FIXED
0.8
People
(Reporter: Fallen, Assigned: Fallen)
Details
(Keywords: perf)
Attachments
(1 file, 1 obsolete file)
2.69 KB,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
Due to the wrong third argument of addEventListener (or removeEventListener), the mousemove and mouseup handlers stay around for each column the event has passed. This doubles the second time the event is dragged. It remained unnoticed, since the respective functions just return if there is no drag column. This patch fixes the event handlers and asserts the drag column must exist for the event handlers, to better find errors in the future. I'm unsure if the better useCapture parameter for add/removeEventListener is true of false. Maybe you know more about that, berend?
Attachment #309894 -
Flags: review?(Berend.Cornelius)
Flags: wanted-calendar0.8+
Comment 1•16 years ago
|
||
Philipp, please don't set the +flag, that one is reserved for release drivers (Daniel, Clint, me). Since we're in release lockdown now, only Daniel will be approving these requests. See http://groups.google.de/group/mozilla.dev.apps.calendar/msg/3b0493be1f9927aa
Flags: wanted-calendar0.8+ → wanted-calendar0.8?
Comment 2•16 years ago
|
||
Comment on attachment 309894 [details] [diff] [review] Fix - v1 I'd like to stick to useCapture and change the removeListener instances to remove with useCapure instead.
Attachment #309894 -
Flags: review?(Berend.Cornelius) → review-
Comment 3•16 years ago
|
||
This would fix leaking handlers and is supposed to be an easy/safe fix => wanted-0.8.
Flags: wanted-calendar0.8? → wanted-calendar0.8+
Assignee | ||
Comment 4•16 years ago
|
||
As discussed, this version fixes the other way around, changing the useCapture parameter of the removeEventListener calls. I left out the ASSERT calls, to make sure no regressions show up.
Attachment #309894 -
Attachment is obsolete: true
Attachment #309995 -
Flags: review?(daniel.boelzle)
Comment 5•16 years ago
|
||
Comment on attachment 309995 [details] [diff] [review] Fix - v2 r=dbo
Attachment #309995 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 6•16 years ago
|
||
Checked in on HEAD and MOZILLA_1_8_BRANCH and SUNBIRD_0_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Comment 7•16 years ago
|
||
Philipp, I couldn't see this checkin on bonsai. -> REOPENED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 8•16 years ago
|
||
Philipp on low bandwidth -- Checked in on HEAD and MOZILLA_1_8_BRANCH and SUNBIRD_0_8_BRANCH -> FIXED
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•