Bug Tracker

Ticket #12837 (closed bug: fixed)

Opened 2 years ago

Last modified 20 months ago

All animations break after zooming a lightbox on the iPad

Reported by: chad.parry@… Owned by: gnarf
Priority: undecided Milestone: 1.8.3
Component: effects Version: 1.8.2
Keywords: Cc:
Blocking: Blocked by:

Description

Repro steps

  1. Lauch Safari on the iPad. I am using Safari version 6.0 with WebKit version 536.26 on iOS version 6.0.
  2. Visit the ColorBox examples:  http://www.jacklmoore.com/colorbox/example1/. This page currently uses jQuery version 1.8.2, and the bug is also reproducible in version 1.7.1.
  3. Open a lightbox by clicking on an example link, such as "Grouped Photo 1."
  4. Pinch to zoom in on the image.
  5. Tap the "X" button in the corner to close the lightbox.

Expected behavior

The lightbox should close.

Observed behavior

The lightbox does not close. All subsequent jQuery events fail.

Technical information

When the lightbox is zoomed on the iPad, it seems that WebKit is allowing JavaScript events to drop. In particular, there is an event that clears the fxNow variable that gets lost. Since the event handler never runs, then the fxNow variable never gets cleared, so it stays at the same value forever, so the jQuery animation thinks that time has stopped. The result is that jQuery spins in a busy loop, and every time it checks the clock it calculates that the time has not changed, so the animation should not advance.

This bug seems to be triggered only when a user pinches to zoom in on a lightbox. The lightbox has to have the CSS style position:fixed to reproduce the bug.

ColorBox workaround

Since this problem has only been observed when using ColorBox, there is also a ColorBox-specific work-around. The work-around is to disable all animations in ColorBox. The easiest way to do that is to find all invocations of fadeTo in jquery.colorbox.js, and change the first argument to 0. However, the real bug is in jQuery, and this ColorBox modification does not address the root cause.

Patch

The fix is to fire another event to clear the fxNow variable. This will allow the animation to always make forward progress, even if WebKit drops events. Other browsers will still work with this fix in place, because clearing fxNow more than once is both safe and cheap.

--- jquery-1.8.2.js	(original)
+++ jquery-1.8.2.js	(fixed)
@@ -8615,6 +8615,9 @@
 			delete tick.elem;
 		}),
 		tick = function() {
+			setTimeout(function() {
+				fxNow = undefined;
+			}, 0 );
 			var currentTime = fxNow || createFxNow(),
 				remaining = Math.max( 0, animation.startTime + animation.duration - currentTime ),
 				percent = 1 - ( remaining / animation.duration || 0 ),

It would be even better to understand exactly what causes the events to be dropped in the first place. That investigation would require more patience and knowledge of WebKit internals than I can muster.

Change History

comment:1 Changed 2 years ago by chad.parry@…

I've created a pull request from my suggested patch:  https://github.com/jquery/jquery/pull/1015.

comment:2 Changed 2 years ago by rwaldron

  • Owner set to chad.parry@…
  • Status changed from new to pending

This is a lot of explanation, but I'd like to see a test case... Historically, jQuery hasn't supported "zoom" related issues because it's been mostly out of scope.

Is this reproducible without human interaction?

comment:3 Changed 2 years ago by dmethvin

there is an event that clears the fxNow variable that gets lost

Do you mean the timer? I think we need to get closer to a root cause before trying to land some sort of fix. This sounds like a Webkit bug that should be reported to them, but a better repro for the specific issue would be helpful.

Perhaps the semantics of setTimeout(fn, 0) has changed? What if you change it to setTimeout(fn, 1) does the problem still happen?

comment:4 Changed 2 years ago by anonymous

@rwaldron: This is not reproducible without human interaction. It only occurs on Safari on the iPad after zooming on a fixed-position lightbox. I don't have any good ideas on how to write a valid unit test.

comment:5 Changed 2 years ago by chad.parry@…

  • Status changed from pending to new

@dmethvin: Sure, I agree that the WebKit behavior is suspect. I'm not a WebKit expert, but I was hoping some other jQuery contributor would know how to resolve a WebKit bug better than I. I don't consider my patch to be a fix as much as a workaround.

I also checked the behavior of setTimeout(fn, 1). The bug still reproduces just like before.

comment:6 Changed 2 years ago by Chad Parry <chad.parry@…>

After a discussion with some other developers, I've created a new pull request:  https://github.com/jquery/jquery/pull/1021. The new version does not fire any spurious clearFxNow events.

comment:7 Changed 2 years ago by gnarf

  • Owner changed from chad.parry@… to gnarf
  • Status changed from new to assigned

Seems like a somewhat "sane" case to make sure each call to jQuery.fx.tick() actually creates a new fxNow.

Chad got us more than halfway there, but I reduced the test case down to something more sane, and the patch to something really simple, only adds a few bytes... See  https://github.com/jquery/jquery/pull/1022

I could see this in general "covering" our ass in case that setTimeout( function() { fxNow = undefined; }, 0 ); gets lost for any reason. Exactly WHY it's getting lost on the iPad after zooming still has me lost however.

comment:8 Changed 2 years ago by Corey Frang

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

Ensure each tick gets it's own fxNow - Fixes #12837 - Thanks @chadparry

Closes gh-1022 Closes gh-1021

Changeset: 781a5c0b78a029b079aae970200d3e4edf543349

comment:9 Changed 2 years ago by Corey Frang

Ensure each tick gets it's own fxNow - Fixes #12837 - Thanks @chadparry

Closes gh-1022 Closes gh-1021 (cherry picked from commit 781a5c0b78a029b079aae970200d3e4edf543349)

Changeset: c56732fd36d45c7c7a95c5f893597c2b755323ad

comment:10 Changed 2 years ago by gnarf

  • Milestone changed from None to 1.8.3

comment:11 Changed 2 years ago by Chad Parry <chad.parry@…>

I confirmed that the fix is working on the iPad. Thanks!

comment:12 Changed 2 years ago by dmethvin

  • Component changed from unfiled to effects

comment:13 Changed 2 years ago by anonymous

How can I download this updated code?

comment:14 Changed 2 years ago by anonymous

This fix made it into the recently-released jQuery 1.8.3. Download it from  http://jquery.com/download/.

comment:15 Changed 22 months ago by dinbror

I'm seeing the same issue with fadeIn/fadeOut in jQuery 1.8.3 but when I use fadeTo instead it works. I expected this issue would solve it for all effects? Am I wrong?

/dinbror

comment:16 Changed 22 months ago by chad.parry@…

@dinbror,

I wouldn't expect that you are seeing the same issue, since the code that caused this issue was removed. You have probably hit similar symptoms from a different issue.

I recommend you file a separate bug. Include a description of what you were doing, your browser version, and a minimal JS example if you can.

comment:17 Changed 21 months ago by shay.grantham@…

Hi All, I've been having the same problem with colorbox and zoom on an ipad. I have tried using jquery 1.8.2 and the latest during testing. My function looks like this:

$('.ajax').colorbox({

onComplete : function() { $(this).colorbox.resize(); }, scrolling: false, innerWidth: "553", escKey: true, transition: "fade", overlayClose: true

});

Still not working! Any suggestions or ideas?

comment:18 Changed 21 months ago by shay.grantham@…

Hi All, I've been having the same problem with colorbox and zoom on an ipad. I have tried using jquery 1.8.2 and the latest during testing. I'm using a fade transition and an onComplete callback to resize the colorbox (due to our image retrieval process not imcluding width and height dimensions).

Still not working! Any suggestions or ideas?

comment:19 Changed 20 months ago by anonymous

@shay.grantham,

You mentioned using jQuery 1.8.2 "and the latest." I just wanted to confirm that you mean you have also tried 1.9.1, which contains the fix for this bug. If you still see a hang with any versions since 1.8.3, then I recommend you file a separate ticket. If you know how to check for JavaScript errors, then attaching those to the ticket would be helpful.

Note: See TracTickets for help on using tickets.