Side navigation
#12837 closed bug (fixed)
Opened November 02, 2012 05:03PM UTC
Closed November 08, 2012 01:25AM UTC
Last modified February 25, 2013 11:05PM UTC
All animations break after zooming a lightbox on the iPad
| Reported by: | chad.parry@overstock.com | Owned by: | gnarf |
|---|---|---|---|
| Priority: | undecided | Milestone: | 1.8.3 |
| Component: | effects | Version: | 1.8.2 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
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.
1. 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.
1. Open a lightbox by clicking on an example link, such as "Grouped Photo 1."
1. Pinch to zoom in on the image.
1. 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.
Attachments (0)
Change History (19)
Changed November 02, 2012 05:58PM UTC by comment:1
Changed November 02, 2012 06:11PM UTC by comment:2
| owner: | → chad.parry@overstock.com |
|---|---|
| status: | new → 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?
Changed November 02, 2012 06:50PM UTC by comment:3
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?
Changed November 05, 2012 04:45PM UTC by comment:4
@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.
Changed November 05, 2012 04:47PM UTC by comment:5
| status: | pending → 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.
Changed November 07, 2012 11:17PM UTC by comment:6
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.
Changed November 08, 2012 12:07AM UTC by comment:7
| owner: | chad.parry@overstock.com → gnarf |
|---|---|
| status: | new → 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.
Changed November 08, 2012 01:25AM UTC by comment:8
| resolution: | → fixed |
|---|---|
| status: | assigned → closed |
Ensure each tick gets it's own fxNow - Fixes #12837 - Thanks @chadparry
Closes gh-1022
Closes gh-1021
Changeset: 781a5c0b78a029b079aae970200d3e4edf543349
Changed November 08, 2012 01:25AM UTC by comment:9
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
Changed November 08, 2012 01:30AM UTC by comment:10
| milestone: | None → 1.8.3 |
|---|
Changed November 08, 2012 04:53PM UTC by comment:11
I confirmed that the fix is working on the iPad. Thanks!
Changed November 11, 2012 08:23PM UTC by comment:12
| component: | unfiled → effects |
|---|
Changed November 16, 2012 10:56PM UTC by comment:13
How can I download this updated code?
Changed November 20, 2012 01:29AM UTC by comment:14
This fix made it into the recently-released jQuery 1.8.3. Download it from http://jquery.com/download/.
Changed January 05, 2013 12:48PM UTC by comment:15
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
Changed January 07, 2013 04:15PM UTC by comment:16
@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.
Changed February 22, 2013 12:59AM UTC by comment:17
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?
Changed February 22, 2013 01:03AM UTC by comment:18
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?
Changed February 25, 2013 11:05PM UTC by comment:19
@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.
I've created a pull request from my suggested patch: https://github.com/jquery/jquery/pull/1015.