Skip to content
This repository

Events: modified touch.js to prevent scroll event listeners failing on re-binding. #5388

Closed
wants to merge 2 commits into from

3 participants

Yosi H. Ghislain Seguin Alexander Schmitz
Yosi H.

Events: modified touch.js to prevent scroll event listeners failing on re-binding.

===The Bug===
The handler binded to the scroll event in the "scrollStart" event's "start" function isn't being unbinded when ".unbind('scroll',someHandler)" is called. This causes the handler to be added multiple times to an element each time the element is binded to the scroll event, even if ".unbind" was called, thus preventing the programmers' supplied handler from listening to the event once a second "bind" was performed.
===The Fix===
I've added a "teardown" function to the event that performs the needed unbinding. (In order to unbind the specific handler I've changed it from an anonymous function to a function property inside the event's namespace).

Cheers :)

Yosi H. yosske Fixed bug:
===The Bug===
The handler binded to the scroll event in the "scrollStart" event's "start" function isn't being unbinded when ".unbind('scroll',someHandler)" is called. This causes the handler to be added multiple times to an element each time it is binded to the scroll event, even if ".unbind" was called, thus preventing the programmers' supplied handler from listening to the event.
===The Fix===
I've added a "teardown" function to the event that performs the needed unbinding. (In order to unbind the specific handler I've changed it from an anonymous function to a function property inside the event's namespace).

Cheers :)
d398582
Ghislain Seguin
Collaborator

@yosske, Thank you for the pull request! It seems like you haven't signed a CLA. Please visit http://contribute.jquery.org/CLA/ and sign it so we can look into your PR.

Thank you.

Yosi H.

I signed a CLA. Please also take into account the recent commit I pushed which fixed an unintentional creation of a Global Variable which was introduced in the first commit.

Thanks!

Alexander Schmitz
Collaborator

Closing this PR because there are conflicts and some of this has been implemented already

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Dec 19, 2012
Yosi H. yosske Fixed bug:
===The Bug===
The handler binded to the scroll event in the "scrollStart" event's "start" function isn't being unbinded when ".unbind('scroll',someHandler)" is called. This causes the handler to be added multiple times to an element each time it is binded to the scroll event, even if ".unbind" was called, thus preventing the programmers' supplied handler from listening to the event.
===The Fix===
I've added a "teardown" function to the event that performs the needed unbinding. (In order to unbind the specific handler I've changed it from an anonymous function to a function property inside the event's namespace).

Cheers :)
d398582
Mar 21, 2013
Yosi H. yosske fixed global "timer" bug ce89aeb
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 26 additions and 22 deletions. Show diff stats Hide diff stats

  1. +26 22 js/events/touch.js
48 js/events/touch.js
@@ -38,15 +38,29 @@ define( [ "jquery", "../jquery.mobile.vmouse", "../jquery.mobile.support.touch"
38 38
39 39 // also handles scrollstop
40 40 $.event.special.scrollstart = {
41   -
  41 +
42 42 enabled: true,
43 43
44   - setup: function() {
  44 + mobileScrollHandler: function( event ) {
  45 +
  46 + if ( !$.event.special.scrollstart.enabled ) {
  47 + return;
  48 + }
  49 +
  50 + if ( !scrolling ) {
  51 + trigger( event, true );
  52 + }
  53 +
  54 + clearTimeout( $.event.special.scrollstart.timer );
  55 + $.event.special.scrollstart.timer = setTimeout( function() {
  56 + trigger( event, false );
  57 + }, 50 );
  58 + },
45 59
  60 + setup: function() {
46 61 var thisObject = this,
47 62 $this = $( thisObject ),
48   - scrolling,
49   - timer;
  63 + scrolling;
50 64
51 65 function trigger( event, state ) {
52 66 scrolling = state;
@@ -54,21 +68,12 @@ define( [ "jquery", "../jquery.mobile.vmouse", "../jquery.mobile.support.touch"
54 68 }
55 69
56 70 // iPhone triggers scroll after a small delay; use touchmove instead
57   - $this.bind( scrollEvent, function( event ) {
58   -
59   - if ( !$.event.special.scrollstart.enabled ) {
60   - return;
61   - }
62   -
63   - if ( !scrolling ) {
64   - trigger( event, true );
65   - }
  71 + $this.bind( scrollEvent, $.event.special.scrollstart.mobileScrollHandler );
  72 + },
66 73
67   - clearTimeout( timer );
68   - timer = setTimeout( function() {
69   - trigger( event, false );
70   - }, 50 );
71   - });
  74 + teardown: function() {
  75 + $(this).unbind( scrollEvent , $.event.special.scrollstart.mobileScrollHandler );
  76 + return false;
72 77 }
73 78 };
74 79
@@ -87,11 +92,10 @@ define( [ "jquery", "../jquery.mobile.vmouse", "../jquery.mobile.support.touch"
87 92 }
88 93
89 94 var origTarget = event.target,
90   - origEvent = event.originalEvent,
91   - timer;
  95 + origEvent = event.originalEvent;
92 96
93 97 function clearTapTimer() {
94   - clearTimeout( timer );
  98 + clearTimeout( $.event.special.tap.timer );
95 99 }
96 100
97 101 function clearTapHandlers() {
@@ -116,7 +120,7 @@ define( [ "jquery", "../jquery.mobile.vmouse", "../jquery.mobile.support.touch"
116 120 .bind( "vclick", clickHandler );
117 121 $( document ).bind( "vmousecancel", clearTapHandlers );
118 122
119   - timer = setTimeout( function() {
  123 + $.event.special.tap.timer = setTimeout( function() {
120 124 triggerCustomEvent( thisObject, "taphold", $.Event( "taphold", { target: origTarget } ) );
121 125 }, $.event.special.tap.tapholdThreshold );
122 126 });

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.