improvements to start and end of swipe gesture animation
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox102 | --- | fixed |
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
Attachments
(1 file)
|
Assignee | |
Comment 1•2 years ago
|
||
The patch for bug 1762875 stops us from calling SwipeTracker::StartAnimation if we are performing a navigation (but still call it if we aren't performing a navigation). This also has the effect of not calling SwipeTracker::SwipeFinished since it is triggered from StartAnimation. This means we don't null out nsBaseWidget::mSwipeTracker from the mWidget.SwipeFinished() call. This means that when the next pan that we want to track as a swipe then when we call nsBaseWidget::TrackScrollEventAsSwipe it will find the existing mSwipeTracker and call mSwipeTracker->CancelSwipe(), which sends a MozSwipeGestureEnd event. It would be okay if we didn't send the MozSwipeGestureEnd event in the case of a succesful navigation, the MozSwipeGesture event can serve the same purpose, but to send a MozSwipeGestureEnd when the next swipe gesture is started does not seem like a good idea.
I noticed this while writing the test for bug 1762875.
To fix it just call SwipeFinished immediately when we get a successful swipe navigation. There are two side effects to consider: nulling out mSwipeTracker on the widget, and sending the MozSwipeGestureEnd event.
Nulling out mSwipeTracker on the widget seems fine, it's only there to track ongoing swipes. Since the current swipe is finished we don't need it anymore. However it brought up an issue where existing tests in widget/tests/browser/browser_test_swipe_gesture.js started failing. A test would finished immediately after a successful swipe navigation, remove the tab, and move on to the next test before the fade out animation could run. If the fade out animation had finished it would have called removeBoxes and removed the dom elements for the visual swipe arrows. So they stick around instead. Removing the tab seems to have disconnected these elements from the DOM. The structure above the swipe elements goes up to the hbox created here https://searchfox.org/mozilla-central/rev/dd404f43c7198b1076fe5d7e05b1e6b1a03bdfeb/browser/base/content/tabbrowser.js#2182 I don't know the tabbrowser code but I'm guessing that gets removed on removing a tab. So in order to fix this we always remove and re-create the boxes when we start a new animation. It should be safe to always do this and remove the isAnimationRunning early exit because we don't call startAnimation when we get a MozSwipeGestureStart event, so we should always want to start fresh when we get that event.
Sending the MozSwipeGestureEnd event means we need to be a bit more careful in stopAnimation. We don't want to do anything in stopAnimation if we are already stopping animation because the fade out will handle it, so we add a early return so that another stopAnimation call (which can come from a MozSwipeGestureEnd) won't cut off the animation prematurely.
Depends on D142820
|
||
Updated•2 years ago
|
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1c67c6bea517 Improvements to start and end of swipe gesture animation. r=hiro
|
||
Comment 3•2 years ago
|
||
| bugherder | ||
Description
•