Intermittent "this.browser.fuzzyZoom is not a function" in AnimatedZoom

RESOLVED FIXED

Status

Firefox for Metro
General
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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+
(Assignee)

Comment 2

5 years ago
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.
Attachment #710822 - Attachment is obsolete: true
Attachment #710929 - Flags: review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/projects/elm/rev/f806df20231e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.