GNOME Bugzilla – Bug 645990
adding anything in "places" to favorites results in bad behavior
Last modified: 2011-06-21 20:24:42 UTC
I have tried this on ubuntu, jhbuild, and fedora 15 and I get the same problem. If I try to add "home" to the favorites bar, I get some strange behavior. I have a hard time getting out of overview and things are just not working correctly. Finally, if you decide to do alt-f2 r, then the whole thing hangs and you have to restart gdm.
When I drag a place (eg. home) to the dash, the whole shell just freeze and I can't do anything until I kill my session and log-in again. Fedora 15 x86_64 gnome 2.91.93
Created attachment 184517 [details] Console output with stack trace
Created attachment 184520 [details] [review] searchDisplay: don't make all SearchResult draggable PlaceSearchProvider/DocSearchProvider should not be draggable
Review of attachment 184520 [details] [review]: This is only a workaround for a bug in searchDisplay - while the dash only accepts dropped application results, dropping docs/places on a particular workspace used to work fine; so we should fix the real bug to make it work again.
Created attachment 184569 [details] [review] search-display: Fix getDragActorSource()/getDragActor() The 'icon' property in search results' meta info has been replaced by a 'createIcon' property, adjust to this change.
I don't think dnd of those items is essential, but the resulting behavior when trying is pretty horrible, so would be nice to see this fixed.
Review of attachment 184569 [details] [review]: Looks safe to me and certainly fixes a serious bug. ::: js/ui/searchDisplay.js @@ +79,2 @@ getDragActorSource: function() { + return this._content; This might not be strictly right. Shouldn't it be content.child not content? It doesn't really matter much because we rescale and we switch pages immediately so it's basically impossible to tell if the alignment was exactly right or not.
Review of attachment 184569 [details] [review]: ::: js/ui/searchDisplay.js @@ +79,2 @@ getDragActorSource: function() { + return this._content; Actually, this isn't right either - it needs to be this._icon.icon noting that this._icon doesn't currently exist, and the code that creates the icon doesn't always run, if provider.createResultActor exists and is used instead. So, maybe this is the best approach with a: // not exactly right, but aligment problems are hard to notice comment.
Assigning to danw for a second review.
Comment on attachment 184569 [details] [review] search-display: Fix getDragActorSource()/getDragActor() yeah, I agree with Owen's review. To fix it completely we'd need the metaInfo to have a getDragActorSource method.
Created attachment 184782 [details] [review] search-display: Fix getDragActorSource()/getDragActor() (In reply to comment #10) > (From update of attachment 184569 [details] [review]) > yeah, I agree with Owen's review. To fix it completely we'd need the metaInfo > to have a getDragActorSource method. Right. Alternatively we could remove getDragActorSource altogether and not get the snapback animation ...
(In reply to comment #10) > (From update of attachment 184569 [details] [review]) > yeah, I agree with Owen's review. To fix it completely we'd need the metaInfo > to have a getDragActorSource method. Can I get another review on this, or should I update the patch with a proper fix now? I'd rather wait with that until freeze break ends ...
Comment on attachment 184782 [details] [review] search-display: Fix getDragActorSource()/getDragActor() >+ // not exactly right, but aligment problems are hard to notice so are typos (marking commit-after-freeze for now since it doesn't yet have release-team approval).
Created attachment 184878 [details] [review] search-display: Fix getDragActorSource()/getDragActor() (In reply to comment #13) > (From update of attachment 184782 [details] [review]) > >+ // not exactly right, but aligment problems are hard to notice > > so are typos Gah - shouldn't rely on copy+paste :(
(In reply to comment #12) > (In reply to comment #10) > > (From update of attachment 184569 [details] [review] [details]) > > yeah, I agree with Owen's review. To fix it completely we'd need the metaInfo > > to have a getDragActorSource method. > > Can I get another review on this, or should I update the patch with a proper > fix now? I'd rather wait with that until freeze break ends ... I think that danw and I are in agreement that your current patch makes the most sense in the short term. I'll send this out in my batch of freeze break emails today.
(In reply to comment #15) > I'll send this out in my batch of freeze break emails > today. Ah, OK - I was about to do it myself, but I guess batching is more r-t friendly ...
Comment on attachment 184878 [details] [review] search-display: Fix getDragActorSource()/getDragActor() Attachment 184878 [details] pushed as 429f809 - search-display: Fix getDragActorSource()/getDragActor() Pushed after r-t approval; leaving bug open for a proper fix after freeze end.
Does this also fix dragging items from the Settings category? I tried that now, and I am unable to drop a search result within Settings onto a workspace and launch it (the drag event is not finished after releasing the mouse button, I have to press Esc to cancel it).
Created attachment 185261 [details] [review] app-search-result: Fix launching on another workspace (In reply to comment #18) > Does this also fix dragging items from the Settings category? No. I'm attaching a fix.
Comment on attachment 185261 [details] [review] app-search-result: Fix launching on another workspace indeed
Comment on attachment 185261 [details] [review] app-search-result: Fix launching on another workspace Attachment 185261 [details] pushed as 09607f6 - app-search-result: Fix launching on another workspace Leaving bug open as of comment 17.
Created attachment 188219 [details] [review] search-display: Try harder to use a correct drag actor source Commit 429f809b7 fixed an exception in getDragActorSource(), but the returned actor is only an approximation (e.g. in contrast to the actual drag actor, it includes the label). Try a bit harder to return the correct actor and only fall back to the approximation when necessary. OK, I admit that I had pretty much forgotten about this bug and the promise to write a proper fix - sorry about that.
Comment on attachment 188219 [details] [review] search-display: Try harder to use a correct drag actor source oops, just noticed this was assigned to me
Attachment 188219 [details] pushed as b9456ca - search-display: Try harder to use a correct drag actor source