GNOME Bugzilla – Bug 787901
DnD: Prevent dividing by zero when calculating the scale factor
Last modified: 2017-09-22 14:51:54 UTC
Probably not a big issue on the shell upstream but on Endless, where DnD is really visible as it's supported on the desktop itself, I've observed lots of warnings when DnD'ing icons around due to having clutter_actor_get_transformed_size() return zero widths for the dragActorSource. Thus, it would be nice to have a zero-check before using that value as the denominator in getRestoreLocation(). From a patch we currently carry around on Endless downstream: https://github.com/endlessm/gnome-shell/commit/856b52c909be654ac2e7660ffb5013857846e857
Created attachment 360051 [details] [review] dnd: Prevent dividing by zero when calculating the scale factor Patch attached
Review of attachment 360051 [details] [review]: ::: js/ui/dnd.js @@ +531,3 @@ [x, y] = this._dragActorSource.get_transformed_position(); let [sourceScaledWidth, sourceScaledHeight] = this._dragActorSource.get_transformed_size(); + scale = sourceScaledWidth ? this._dragActor.width / sourceScaledWidth : 1; This looks wrong - if the source actor has been scaled to 0, then I think it makes more sense to shrink the drag actor to 0 when snapping back rather than keeping it unscaled and disappear abruptly at the end of the transition.
Good point, that works better indeed
Created attachment 360256 [details] [review] dnd: Prevent dividing by zero when calculating the scale factor
Review of attachment 360256 [details] [review]: There's an accidental submodule update in the patch, otherwise LGTM. The commit message could be better though - "moving icons aroung(sic!) the desktop" doesn't actually apply to upstream. Something like: This ensures that we snap back to the correct size when the source actor has been scaled to 0.
(In reply to Florian Müllner from comment #5) > Review of attachment 360256 [details] [review] [review]: > > There's an accidental submodule update in the patch, otherwise LGTM. > > The commit message could be better though - "moving icons aroung(sic!) the > desktop" doesn't actually apply to upstream. Something like: > > This ensures that we snap back to the correct size when the > source actor has been scaled to 0. Oops! Fixed those minor issues and pushed to master: https://git.gnome.org/browse/gnome-shell/commit/?id=703187e99620de085978954fe3a3004676de1046