New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event bubbling issue with swipe gesture #6262

Closed
atuttle opened this Issue Jul 31, 2013 · 7 comments

Comments

Projects
None yet
4 participants
@atuttle

atuttle commented Jul 31, 2013

I've posted this issue on Stack Overflow but it's not getting much traction there. For all I can tell, this is either some weird edge case of how the swipe gesture is implemented or a bug in event bubbling.

What it boils down to the following:

  • I have a page with a horizontal-scrollable section (tab bar, ala Android Holo tabs)
  • listening for swipe on the page element because the content may not always fill the screen vertically (see the SO post for more info on this)
  • swipe gesture listener added to tab bar with event.stopPropagation()

But it appears that the swipe gesture is still bubbling up despite my request for it not to.

I tried my hand at creating a jsbin demonstrating the problem: http://jsbin.com/ofuhaw/113/

I was having trouble using an in-page selector to show the current count so I just went with console.log instead.

Just shrink your browser width so that the tab bar gets a scrollbar, pop open your console, and do swipes over both the page (below the <hr/>) and on the tabs themselves. You should see that the counter increments even when you swipe over the tab bar, and -- if you try this page on your phone -- the tabs don't move.

For contrast, here's a duplicate of that jsbin, with the swipe gestures completely commented out: http://jsbin.com/ofuhaw/115

"swiping" over the tabs scrolls them horizontally. (I'm having a lot of issues with jsbin; one of which being that this seems to only scroll the first time, but take my word for it, this code outside of jsbin does scroll just fine with swipe gestures removed)

@arschmitz

This comment has been minimized.

Show comment
Hide comment
@arschmitz

arschmitz Jul 31, 2013

Member

@atuttle you cannot stop propagation on delegated events see jquery docs note http://api.jquery.com/event.stopPropagation/

Member

arschmitz commented Jul 31, 2013

@atuttle you cannot stop propagation on delegated events see jquery docs note http://api.jquery.com/event.stopPropagation/

@arschmitz arschmitz closed this Jul 31, 2013

@arschmitz arschmitz reopened this Jul 31, 2013

@arschmitz

This comment has been minimized.

Show comment
Hide comment
@arschmitz

arschmitz Jul 31, 2013

Member

Sorry your event is not delegated reopened

Member

arschmitz commented Jul 31, 2013

Sorry your event is not delegated reopened

@atuttle

This comment has been minimized.

Show comment
Hide comment
@atuttle

atuttle Jul 31, 2013

Thanks for reopening; but I did learn something new! (... that you can't stop delegated events; thanks for that!)

atuttle commented Jul 31, 2013

Thanks for reopening; but I did learn something new! (... that you can't stop delegated events; thanks for that!)

@atuttle

This comment has been minimized.

Show comment
Hide comment
@atuttle

atuttle Aug 1, 2013

Here's a jsFiddle that demonstrates how it should, in theory, work. This is the same HTML and CSS as my JSBin's above, but with absolutely no JS included other than the libraries.

http://jsfiddle.net/atuttle/3TW64/embedded/result/

You can open that up on a mobile device, then pinch-to-zoom in so that the tabs are scrollable, and swipe back and forth across them. This is the behavior that is obstructed by using the swipe gesture.

atuttle commented Aug 1, 2013

Here's a jsFiddle that demonstrates how it should, in theory, work. This is the same HTML and CSS as my JSBin's above, but with absolutely no JS included other than the libraries.

http://jsfiddle.net/atuttle/3TW64/embedded/result/

You can open that up on a mobile device, then pinch-to-zoom in so that the tabs are scrollable, and swipe back and forth across them. This is the behavior that is obstructed by using the swipe gesture.

@atuttle

This comment has been minimized.

Show comment
Hide comment
@atuttle

atuttle Aug 1, 2013

I found a workaround, but I'd still prefer a proper solution.

atuttle commented Aug 1, 2013

I found a workaround, but I'd still prefer a proper solution.

@snielson-dyn

This comment has been minimized.

Show comment
Hide comment
@snielson-dyn

snielson-dyn Jan 6, 2014

I ran into a problem with my events propagating as well on swipe events. I believe this is a bug introduced into the jquerymobile 1.4 release. I've tested the code with jquerymobile 1.3.2 and I get my events to stop propagating. On 1.4 they do not. Here are two fiddles that show the behavior.

http://jsfiddle.net/ta3gz/1/ - jQuery Mobile 1.3.2 which stops the bubbling

http://jsfiddle.net/nP5Z8/ - jQuery Mobile 1.4.0 which does not stop the bubbling.

I don't have a lot of cycles to debug this but I'll try looking at the unminified version of 1.4 and try to see what's going on.

snielson-dyn commented Jan 6, 2014

I ran into a problem with my events propagating as well on swipe events. I believe this is a bug introduced into the jquerymobile 1.4 release. I've tested the code with jquerymobile 1.3.2 and I get my events to stop propagating. On 1.4 they do not. Here are two fiddles that show the behavior.

http://jsfiddle.net/ta3gz/1/ - jQuery Mobile 1.3.2 which stops the bubbling

http://jsfiddle.net/nP5Z8/ - jQuery Mobile 1.4.0 which does not stop the bubbling.

I don't have a lot of cycles to debug this but I'll try looking at the unminified version of 1.4 and try to see what's going on.

@ghost ghost assigned gabrielschulhof Jan 15, 2014

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Jan 15, 2014

Contributor

I have debugged it.

The fundamental problem is this: If you attach a swipe handler to an element and you attach another swipe handler to a parent of that element, then two swipe events will be generated for every mouse gesture. That's because mouse events bubble, and so a sequence of mouse events will be processed by the swipe calculator attached to the element, and the same sequence of mouse events will be processed again by the swipe calculator attached to the parent.

In 1.3.2, we were triggering swipe on the element from which it originated and we were allowing it to bubble. Thus, both swipe events were being triggered on the element that received the original mousedown, so they were both getting passed to the handler which calls stopPropagation, so both events were prevented from bubbling.

In 1.4.0, we trigger swipe on the element to which the swipe handler is attached, not on the element that received the original mousedown and we're not allowing the swipe event to bubble. Thus, only one swipe event is being received by the handler which calls stopPropagation, because the other swipe event originates "above its head", at the parent, so it never gets a chance to call stopPropagation on it. The fact that the parent receives a swipe event with a non-prevented default is not due to the inner swipe event bubbling (because we don't allow that anymore), but because it is receiving a swipe event that was generated especially for it.

The correct solution IMO is to use a sequence of mouse events for the generation of at most one swipe event on the innermost element, and to allow that swipe event to bubble. What this means is that we have to set a global flag indicating that a swipe calculation is already in progress, so only the first swipe calculator will actually do any calculating, and all other swipe calculators will bail.

Contributor

gabrielschulhof commented Jan 15, 2014

I have debugged it.

The fundamental problem is this: If you attach a swipe handler to an element and you attach another swipe handler to a parent of that element, then two swipe events will be generated for every mouse gesture. That's because mouse events bubble, and so a sequence of mouse events will be processed by the swipe calculator attached to the element, and the same sequence of mouse events will be processed again by the swipe calculator attached to the parent.

In 1.3.2, we were triggering swipe on the element from which it originated and we were allowing it to bubble. Thus, both swipe events were being triggered on the element that received the original mousedown, so they were both getting passed to the handler which calls stopPropagation, so both events were prevented from bubbling.

In 1.4.0, we trigger swipe on the element to which the swipe handler is attached, not on the element that received the original mousedown and we're not allowing the swipe event to bubble. Thus, only one swipe event is being received by the handler which calls stopPropagation, because the other swipe event originates "above its head", at the parent, so it never gets a chance to call stopPropagation on it. The fact that the parent receives a swipe event with a non-prevented default is not due to the inner swipe event bubbling (because we don't allow that anymore), but because it is receiving a swipe event that was generated especially for it.

The correct solution IMO is to use a sequence of mouse events for the generation of at most one swipe event on the innermost element, and to allow that swipe event to bubble. What this means is that we have to set a global flag indicating that a swipe calculation is already in progress, so only the first swipe calculator will actually do any calculating, and all other swipe calculators will bail.

gabrielschulhof added a commit that referenced this issue Jan 15, 2014

Swipe event: Trigger only one event per mouse sequence and let it bubble
The element to which a mousedown handler is attached by the swipe setup() code
takes ownership of the next swipe event. This means that, if the same mousedown
event triggers other swipe setup() -attached mousedown handlers as it bubbles,
the subsequent ones will bail. Thus, if a swipe event results from the mousedown
then such a swipe event will be triggered on the deepest element with an
attached swipe handler, and it will be allowed to bubble from there. This
provides the tools that developers can use to avoid scenarios such as the one
mentioned in the referenced issue.

Fixes gh-6262

gabrielschulhof added a commit that referenced this issue Jan 15, 2014

Swipe event: Trigger only one event per mouse sequence and let it bubble
The element to which a mousedown handler is attached by the swipe setup() code
takes ownership of the next swipe event. This means that, if the same mousedown
event triggers other swipe setup() -attached mousedown handlers as it bubbles,
the subsequent ones will bail. Thus, if a swipe event results from the mousedown
then such a swipe event will be triggered on the deepest element with an
attached swipe handler, and it will be allowed to bubble from there. This
provides the tools that developers can use to avoid scenarios such as the one
mentioned in the referenced issue.

Fixes gh-6262

gabrielschulhof added a commit that referenced this issue Jan 16, 2014

Swipe event: Trigger only one event per mouse sequence and let it bubble
The element to which a mousedown handler is attached by the swipe setup() code
takes ownership of the next swipe event. This means that, if the same mousedown
event triggers other swipe setup() -attached mousedown handlers as it bubbles,
the subsequent ones will bail. Thus, if a swipe event results from the mousedown
then such a swipe event will be triggered on the deepest element with an
attached swipe handler, and it will be allowed to bubble from there. This
provides the tools that developers can use to avoid scenarios such as the one
mentioned in the referenced issue.

Fixes gh-6262

gabrielschulhof added a commit that referenced this issue Jan 21, 2014

Swipe event: Trigger only one event per mouse sequence and let it bubble
The element to which a mousedown handler is attached by the swipe setup() code
takes ownership of the next swipe event. This means that, if the same mousedown
event triggers other swipe setup() -attached mousedown handlers as it bubbles,
the subsequent ones will bail. Thus, if a swipe event results from the mousedown
then such a swipe event will be triggered on the deepest element with an
attached swipe handler, and it will be allowed to bubble from there. This
provides the tools that developers can use to avoid scenarios such as the one
mentioned in the referenced issue.

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