Created attachment 710822 [details] [diff] [review] patch If a tab is closed while AnimatedZoom is in the middle of an animation, AnimatedZoom.updateTo throws an exception because it tries to call an XBL method on an element that no longer has a binding. This shows up sometimes in Metro tests. We should just cancel the animation in this case. I tested that AnimatedZoom still works as expected, and the browser_onscreen_keyboard.js test which uses it is still passing. (Note: AnimatedZoom is not actually used for zooming right now, but it's still used for animated scrolling in FormHelperUI. It will probably go away completely in the future, replaced by AsyncPanZoomController.)
Attachment #710822 - Flags: review?(mark.finkle)
Comment on attachment 710822 [details] [diff] [review] patch I assume we don't need to do anything special for the other calls to AnimatedZoom.updateTo Also, it looks like AnimatedZoom.finish could have "reset" called twice: one at the top of the function via updateTo and once at the bottom. Is this likely? If so, is it a problem?
Attachment #710822 - Flags: review?(mark.finkle) → review+
Created attachment 710929 [details] [diff] [review] patch Good catch -- in the rare case where a tab is destroyed at *exactly* the moment when finish() is about to be called, then we'd get similar exceptions in finish(). We should bail out of it. This also avoids calling _reset twice, which would lead to (probably harmless) duplicate events. We can assume the updateTo call in animateTo succeeds, because there's no chance for the browser to disappear between the call to start() and the call to updateTo(). Carrying r=mfinkle.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.