GNOME Bugzilla – Bug 644642
xdnd: Dragging item between workspaces in overview mode skips workspaces in the middle
Last modified: 2011-03-14 21:03:39 UTC
I was trying out GNOME shell in the latest OpenSUSE Live CD (0.1.0). I tried to perform the task of dragging a piece of text in a window in workspace 1, taking it to the top-left corner to expose the activities overview, and then trying to drop it onto another window in workspace 2. I found that it was impossible to scroll and stop at the second workspace, instead, the view rapidly scrolled down to the last workspace. While dragging an object, I could only move between the first and last workspace, and found myself unable to stop at any workspace in between, which I believe is a bug. I also found that trying to choose a workspace to drop to through the selector on the right of the screen only works if a part of a window is visible on the partly exposed workspace selector. While dragging an object over the workspace selector, the widget should expose itself completely.
Reproduced both problems.
Created attachment 183288 [details] [review] XDND: Fix dragMonitor leak in WorkspacesDisplay WorkspacesDisplay removes its dragMonitor in _dragEnd, but this was never called in when a xdnd drag ended causing dragMonitors to stack up and handling events multiple times. Fix that by making sure that _dragEnd is called when xdnd ends.
(In reply to comment #0) > I was trying out GNOME shell in the latest OpenSUSE Live CD (0.1.0). I tried to > perform the task of dragging a piece of text in a window in workspace 1, taking > it to the top-left corner to expose the activities overview, and then trying to > drop it onto another window in workspace 2. I found that it was impossible to > scroll and stop at the second workspace, instead, the view rapidly scrolled > down to the last workspace. While dragging an object, I could only move between > the first and last workspace, and found myself unable to stop at any workspace > in between, which I believe is a bug. This one should be fixed by the attached patch. > I also found that trying to choose a workspace to drop to through the selector > on the right of the screen only works if a part of a window is visible on the > partly exposed workspace selector. While dragging an object over the workspace > selector, the widget should expose itself completely. This should be fixed once we land the patch from 643945
(In reply to comment #3) > > I also found that trying to choose a workspace to drop to through the selector > > on the right of the screen only works if a part of a window is visible on the > > partly exposed workspace selector. While dragging an object over the workspace > > selector, the widget should expose itself completely. > > This should be fixed once we land the patch from 643945 Ah, forgot that hadn't landed.
Review of attachment 183288 [details] [review]: Hmm, not obvious to me ::: js/ui/overview.js @@ +266,3 @@ this.hideTemporarily(); this._lastHoveredWindow = null; + this.endItemDrag(dragEvent.targetActor); so in this case we emit item-drag-end ::: js/ui/workspacesView.js @@ +109,3 @@ + + this._xdndDragEndId = Main.xdndHandler.connect('drag-end', + Lang.bind(this, this._dragEnd)); Here we have paired connections to item-drag-begin/item-drag-end and window-drag-begin/window-drag-end - if xdnd shows up as item-drag-begin, shouldn't it uniformly show up as item-drag-end?
(In reply to comment #5) > Review of attachment 183288 [details] [review]: > > Hmm, not obvious to me > > ::: js/ui/overview.js > @@ +266,3 @@ > this.hideTemporarily(); > this._lastHoveredWindow = > null; > + > this.endItemDrag(dragEvent.targetActor); > > so in this case we emit item-drag-end > > ::: js/ui/workspacesView.js > @@ +109,3 @@ > + > + this._xdndDragEndId = Main.xdndHandler.connect('drag-end', > + Lang.bind(this, > this._dragEnd)); > > Here we have paired connections to item-drag-begin/item-drag-end and > window-drag-begin/window-drag-end - if xdnd shows up as item-drag-begin, > shouldn't it uniformly show up as item-drag-end? Yeah makes sense.
Created attachment 183354 [details] [review] XDND: Fix dragMonitor leak in WorkspacesDisplay WorkspacesDisplay removes its dragMonitor in _dragEnd, but this was never called in when a xdnd drag ended causing dragMonitors to stack up and handling events multiple times. Fix that by making sure that _dragEnd is called when xdnd ends.
Review of attachment 183354 [details] [review]: ::: js/ui/overview.js @@ +267,3 @@ this.hideTemporarily(); this._lastHoveredWindow = null; + this.endItemDrag(); Why don't we drag-end from xdndHandler in this case? if we do, I don't think users should have to worry about unpaired item-drag-begin/item-drag-end calls - we should avoid doing item-begin-drag/item-end-drag/item-end-drag. If the issue is that Workspaces doesn't remove it's monitor if it's destroyed before item-drag-end is emitted, I think it would be cleanest to make it clean up properly if destroyed during a drag - if (_inDrag) this._dragEnd() or whatever.
Created attachment 183384 [details] [review] XDND: Fix dragMonitor leak in WorkspacesDisplay WorkspacesDisplay removes its dragMonitor in _dragEnd, but this was never called in when a xdnd drag ended causing dragMonitors to stack up and handling events multiple times. Fix that by making sure that _dragEnd is called when xdnd ends. ----- (In reply to comment #8) > Review of attachment 183354 [details] [review]: > > ::: js/ui/overview.js > @@ +267,3 @@ > this.hideTemporarily(); > this._lastHoveredWindow = > null; > + this.endItemDrag(); > > Why don't we drag-end from xdndHandler in this case? if we do, I don't think > users should have to worry about unpaired item-drag-begin/item-drag-end calls - > we should avoid doing item-begin-drag/item-end-drag/item-end-drag. If the issue > is that Workspaces doesn't remove it's monitor if it's destroyed before > item-drag-end is emitted, I think it would be cleanest to make it clean up > properly if destroyed during a drag - if (_inDrag) this._dragEnd() or whatever. No reason ... and it makes sense to not emit "item-drag-end" twice.
Review of attachment 183384 [details] [review]: Looks good (I've assuming you've retested the original bug with the new revision)
Attachment 183384 [details] pushed as 29d473f - XDND: Fix dragMonitor leak in WorkspacesDisplay