From e0414e7cc7cc9e997659f297637f35d9202b6dab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 5 Dec 2017 22:41:17 +0100
Subject: [PATCH] dnd: Nullify _dragActor after we've destroyed it, and avoid
invalid access
We need to avoid that we use the _dragActor instance after that it has
been destroyed or we'll get errors. We now set it to null when this
happens, protecting any access to that.
Add a DragState enum-like object to keep track of the state
instead of using booleans.
Remove duplicated handler on 'destroy' and just use a generic one.
https://bugzilla.gnome.org/show_bug.cgi?id=791233
---
js/ui/dnd.js | 65 +++++++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 26 deletions(-)
diff --git a/js/ui/dnd.js b/js/ui/dnd.js
index a38607c24..431c60d6c 100644
--- a/js/ui/dnd.js
+++ b/js/ui/dnd.js
@@ -27,6 +27,12 @@ var DragMotionResult = {
CONTINUE: 3
};
+var DragState = {
+ INIT: 0,
+ DRAGGING: 1,
+ CANCELLED: 2,
+};
+
var DRAG_CURSOR_MAP = {
0: Meta.Cursor.DND_UNSUPPORTED_TARGET,
1: Meta.Cursor.DND_COPY,
@@ -78,6 +84,8 @@ var _Draggable = new Lang.Class({
dragActorOpacity: undefined });
this.actor = actor;
+ this._dragState = DragState.INIT;
+
if (!params.manualMode) {
this.actor.connect('button-press-event',
this._onButtonPress.bind(this));
@@ -88,7 +96,7 @@ var _Draggable = new Lang.Class({
this.actor.connect('destroy', () => {
this._actorDestroyed = true;
- if (this._dragInProgress && this._dragCancellable)
+ if (this._dragState == DragState.DRAGGING && this._dragCancellable)
this._cancelDrag(global.get_current_time());
this.disconnectAll();
});
@@ -100,7 +108,6 @@ var _Draggable = new Lang.Class({
this._dragActorOpacity = params.dragActorOpacity;
this._buttonDown = false; // The mouse button has been pressed and has not yet been released.
- this._dragInProgress = false; // The drag has been started, and has not been dropped or cancelled yet.
this._animationInProgress = false; // The drag is over and the item is in the process of animating to its original position (snapping back or reverting).
this._dragCancellable = true;
@@ -206,9 +213,10 @@ var _Draggable = new Lang.Class({
(event.type() == Clutter.EventType.TOUCH_END &&
global.display.is_pointer_emulating_sequence(event.get_event_sequence()))) {
this._buttonDown = false;
- if (this._dragInProgress) {
+ if (this._dragState == DragState.DRAGGING) {
return this._dragActorDropped(event);
- } else if (this._dragActor != null && !this._animationInProgress) {
+ } else if ((this._dragActor != null || this._dragState == DragState.CANCELLED) &&
+ !this._animationInProgress) {
// Drag must have been cancelled with Esc.
this._dragComplete();
return Clutter.EVENT_STOP;
@@ -222,14 +230,14 @@ var _Draggable = new Lang.Class({
} else if (event.type() == Clutter.EventType.MOTION ||
(event.type() == Clutter.EventType.TOUCH_UPDATE &&
global.display.is_pointer_emulating_sequence(event.get_event_sequence()))) {
- if (this._dragInProgress) {
+ if (this._dragActor && this._dragState == DragState.DRAGGING) {
return this._updateDragPosition(event);
- } else if (this._dragActor == null) {
+ } else if (this._dragActor == null && this._dragState != DragState.CANCELLED) {
return this._maybeStartDrag(event);
}
// We intercept KEY_PRESS event so that we can process Esc key press to cancel
// dragging and ignore all other key presses.
- } else if (event.type() == Clutter.EventType.KEY_PRESS && this._dragInProgress) {
+ } else if (event.type() == Clutter.EventType.KEY_PRESS && this._dragState == DragState.DRAGGING) {
let symbol = event.get_key_symbol();
if (symbol == Clutter.Escape) {
this._cancelDrag(event.get_time());
@@ -265,7 +273,7 @@ var _Draggable = new Lang.Class({
*/
startDrag(stageX, stageY, time, sequence) {
currentDraggable = this;
- this._dragInProgress = true;
+ this._dragState = DragState.DRAGGING;
// Special-case St.Button: the pointer grab messes with the internal
// state, so force a reset to a reasonable state here
@@ -342,6 +350,13 @@ var _Draggable = new Lang.Class({
Shell.util_set_hidden_from_pick(this._dragActor, true);
}
+ this._dragActorDestroyId = this._dragActor.connect('destroy', () => {
+ // Cancel ongoing animation (if any)
+ this._finishAnimation();
+
+ this._dragActor = null;
+ this._dragState = DragState.CANCELLED;
+ });
this._dragOrigOpacity = this._dragActor.opacity;
if (this._dragActorOpacity != undefined)
this._dragActor.opacity = this._dragActorOpacity;
@@ -500,7 +515,7 @@ var _Draggable = new Lang.Class({
event.get_time())) {
// If it accepted the drop without taking the actor,
// handle it ourselves.
- if (this._dragActor.get_parent() == Main.uiGroup) {
+ if (this._dragActor && this._dragActor.get_parent() == Main.uiGroup) {
if (this._restoreOnSuccess) {
this._restoreDragActor(event.get_time());
return true;
@@ -508,7 +523,7 @@ var _Draggable = new Lang.Class({
this._dragActor.destroy();
}
- this._dragInProgress = false;
+ this._dragState = DragState.INIT;
global.screen.set_cursor(Meta.Cursor.DEFAULT);
this.emit('drag-end', event.get_time(), true);
this._dragComplete();
@@ -557,20 +572,22 @@ var _Draggable = new Lang.Class({
_cancelDrag(eventTime) {
this.emit('drag-cancelled', eventTime);
- this._dragInProgress = false;
- let [snapBackX, snapBackY, snapBackScale] = this._getRestoreLocation();
+ let wasCancelled = (this._dragState == DragState.CANCELLED);
+ this._dragState = DragState.CANCELLED;
- if (this._actorDestroyed) {
+ if (this._actorDestroyed || wasCancelled) {
global.screen.set_cursor(Meta.Cursor.DEFAULT);
if (!this._buttonDown)
this._dragComplete();
this.emit('drag-end', eventTime, false);
- if (!this._dragOrigParent)
+ if (!this._dragOrigParent && this._dragActor)
this._dragActor.destroy();
return;
}
+ let [snapBackX, snapBackY, snapBackScale] = this._getRestoreLocation();
+
this._animateDragEnd(eventTime,
{ x: snapBackX,
y: snapBackY,
@@ -581,7 +598,7 @@ var _Draggable = new Lang.Class({
},
_restoreDragActor(eventTime) {
- this._dragInProgress = false;
+ this._dragState = DragState.INIT;
let [restoreX, restoreY, restoreScale] = this._getRestoreLocation();
// fade the actor back in at its original location
@@ -596,12 +613,6 @@ var _Draggable = new Lang.Class({
_animateDragEnd(eventTime, params) {
this._animationInProgress = true;
- // finish animation if the actor gets destroyed
- // during it
- this._dragActorDestroyId =
- this._dragActor.connect('destroy',
- this._finishAnimation.bind(this));
-
params['opacity'] = this._dragOrigOpacity;
params['transition'] = 'easeOutQuad';
params['onComplete'] = this._onAnimationComplete;
@@ -624,9 +635,6 @@ var _Draggable = new Lang.Class({
},
_onAnimationComplete(dragActor, eventTime) {
- dragActor.disconnect(this._dragActorDestroyId);
- this._dragActorDestroyId = 0;
-
if (this._dragOrigParent) {
Main.uiGroup.remove_child(this._dragActor);
this._dragOrigParent.add_actor(this._dragActor);
@@ -641,7 +649,7 @@ var _Draggable = new Lang.Class({
},
_dragComplete() {
- if (!this._actorDestroyed)
+ if (!this._actorDestroyed && this._dragActor)
Shell.util_set_hidden_from_pick(this._dragActor, false);
this._ungrabEvents();
@@ -652,7 +660,12 @@ var _Draggable = new Lang.Class({
this._updateHoverId = 0;
}
- this._dragActor = undefined;
+ if (this._dragActor) {
+ this._dragActor.disconnect(this._dragActorDestroyId);
+ this._dragActor = null;
+ }
+
+ this._dragState = DragState.INIT;
currentDraggable = null;
}
});
--
2.19.1