gh-14984: Fix tab strip staying offset when a space swipe is interrupted - #14985
gh-14984: Fix tab strip staying offset when a space swipe is interrupted#14985obi12341 wants to merge 5 commits into
Conversation
…wipe is interrupted
dosubot
Bot
added
size:M
|
Have you tested this fix? |
| // Cleanup only runs on MozSwipeGestureEnd, which never arrives when the window | ||
| // stops receiving the rest of the gesture. The strip then keeps its offset and | ||
| // the tab strip its disabled pointer events, see gh-13703 for those. | ||
| #attachGestureAbortTriggers() { |
There was a problem hiding this comment.
Not sure, you marked this as resolved but the function is still here?
There was a problem hiding this comment.
Sorry, I read your comment as being about the comment block above the function and
removed that.
Do you mean the helper itself? I can fold those two addEventListener calls into
#attachWorkspaceSwipeGestures so there is no extra function, same behaviour.
There was a problem hiding this comment.
Yes, I meant the helper itself. Since the chrome window wont ever call visibilitychange and deactivate is already handled by the swipe manager.
| for (const container of document.querySelectorAll( | ||
| "#zen-essentials .zen-workspace-tabs-section" | ||
| )) { | ||
| container.style.removeProperty("transform"); |
There was a problem hiding this comment.
This doesn't seem like a good idea? Why did you change this? We should animate back to the original space. Why wasn't onSwipeGestureAnimationEnd working out?
There was a problem hiding this comment.
You're right about animating back. Switched to _cancelSwipeAnimation() and dropped both
the manual _organizeWorkspaceStripLocations call and the essentials loop.
On onSwipeGestureAnimationEnd: in this scenario it never runs at all, because it is only
wired to MozSwipeGestureEnd, and the widget never sends that once the window stops
receiving the gesture.
…f resetting the strip location
dosubot
Bot
added
size:S
Yes. Measured on a build of this branch with a clean profile, driving the gesture with |
| // Cleanup only runs on MozSwipeGestureEnd, which never arrives when the window | ||
| // stops receiving the rest of the gesture. The strip then keeps its offset and | ||
| // the tab strip its disabled pointer events, see gh-13703 for those. | ||
| #attachGestureAbortTriggers() { |
There was a problem hiding this comment.
Not sure, you marked this as resolved but the function is still here?
|
It all looks very ai generated and half of the stuff doesnt make much sense |
|
@mr-cheffy The PR fixes a real bug and includes a test that demonstrates it. If you don't understand the bug itself you could have asked. I asked how you wanted the code structured so it would be acceptable to you. I got no answer for a week, then a close. |
|
@obi12341 Im truly sorry, I missed that part. Seems like the comments got marked as resolved and hence stayed collapsed / hidden. I'll reopen and answer. |
2 participants
Closes #14984
ZenSpacesSwipeonly cleans up inonSwipeGestureAnimationEnd(), which is wired toMozSwipeGestureEnd. When a gesture is abandoned — the window is deactivated, or a popuptakes over — that event never arrives and nothing recovers the state:
translateX(...)applied by_handleSwipeUpdatestays on the strip, andswipe-gesture="true"stays ondocumentElement, keeping:root[swipe-gesture] #tabbrowser-arrowscrollbox { pointer-events: none }([1.19.13b] Workspace swiping will ocassionally be stuck. #13703)active, which is what stops the tab list from scrolling.
This adds
#abortGesture(), which puts the strip back on the current space, clears theessentials transforms and runs the existing cleanup. It is triggered on
deactivateand onvisibilitychangewhile hidden._popupOpenHandlernow goes through it as well — that pathhad the same defect, since it reset the swipe state but never the transform.