After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 579137 - Cannot rearange playlists longer than one screen size
Cannot rearange playlists longer than one screen size
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: User Interface
0.12.x
Other All
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
: 579704 582339 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-04-16 10:36 UTC by Mislav Covran
Modified: 2009-06-13 14:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fixes the scrolling bug. (1.00 KB, patch)
2009-05-16 05:33 UTC, Ryan Hughes
committed Details | Review

Description Mislav Covran 2009-04-16 10:36:36 UTC
Please describe the problem:
It is basically impossible to rearange a playlist if it doesn't fit on the screen. As soon as I try to drag an item, I get thrown to the top of the playlist and I can't scroll down.

Steps to reproduce:
1. Create a playlist with enough songs to cover a few screen sizes (at least 2 or 3) or you might not notice the effect
2. Scroll to the bottom of the playlist
3. Try to drag a song and drop it just a few places up

Actual results:
Playlist scrolls all the way up to the top.

Expected results:
It should behave like any other drag-and-drop area that involves scrolling. If I move the mouse to the top of the area it should move up, and if I move it to the bottom of the area it should scroll down.

Does this happen every time?
Yes.

Other information:
It worked with previous versions of Rhythmbox I used.
Comment 1 Jonathan Matthew 2009-04-21 10:28:22 UTC
This is strange.  The drag and drop code in rhythmbox has not changed in years.
I suspect that a change in gtk has introduced this.
Comment 2 Jonathan Matthew 2009-04-21 10:49:07 UTC
*** Bug 579704 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Matthew 2009-05-13 00:46:13 UTC
*** Bug 582339 has been marked as a duplicate of this bug. ***
Comment 4 Ryan Hughes 2009-05-16 05:33:19 UTC
Created attachment 134747 [details] [review]
Fixes the scrolling bug.

I figured it out.  The scroll_row_timeout used to convert the widget coordinates to tree coordinates.  However, the gtk reference manual now says that gtk_tree_view_widget_to_tree_coords is deprecated.  It refers readers to gtk_tree_view_convert_widget_to_bin_window_coords, but fails to mention that this only gets us halfway there.

The bin_window coordinates then need to be converted to tree coordinates, using gtk_tree_view_convert_bin_window_to_tree_coords.

This patch adds that conversion.  Adding songs to playlists and dragging stuff around is now back to normal.
Comment 5 Jonathan Matthew 2009-05-16 14:52:31 UTC
Great, thanks for figuring that out.  Looks like I was totally wrong in my first comment..

Any thoughts on why this doesn't seem to affect the other places we call gtk_tree_view_convert_widget_to_bin_coords?  We use it in the cell renderer classes used to render the group expanders in the source list, and the error/paused/playing icons and ratings in track lists.
Comment 6 Ryan Hughes 2009-05-17 17:29:41 UTC
Yeah, it seems that the cell_area rectangle that's passed to the gtk_cell_renderer_activate (and derived functions) is already in bin_window coordinates.  I determined this experimentally, by printing out the (mouse_x, mouse_y) before and after the conversion from widget to bin_window coordinates, and then printing out the cell_area (x, y).  I clicked in the top-left corner of a rating cell, and found that the cell_area was in bin_window coordinates.

This is what the doc has to say about it:
cell_area :
	cell area as passed to gtk_cell_renderer_render() 

So, probably if I knew more about gtk, I would immediately understand that gtk_cell_renderer_renderer expects the cell_area to be passed in in bin_window coordinates, and I would immediately infer that the cell_area argument of the activate function is also supposed to be in bin_window coordinates.  Still, I wish that the docs would be more explicit about this kind of thing.

Anyway, that's why converting the mouse to bin_window coordinates is the right thing to do in this case.
Comment 7 Jonathan Matthew 2009-05-17 23:33:00 UTC
OK, thanks.  I've committed the patch.
Comment 8 radu tanasescu 2009-06-12 16:56:58 UTC
(In reply to comment #4)
> Created an attachment (id=134747) [edit]
> Fixes the scrolling bug.
> 
[...]
> 
> This patch adds that conversion.  Adding songs to playlists and dragging stuff
> around is now back to normal.

Dear Ryan

I have the same Rhythmbox problem on my laptop.
OS: Ubuntu Jaunty AMD 64
I would like to know how to apply the patch

Please excuse my lack of experience in Linux commands and
thank you very much in advance for your help.

All the best,
radu

Comment 9 Hew McLachlan 2009-06-13 06:52:39 UTC
Radu, the problem is fixed in the latest version of Rhythmbox. For Ubuntu see https://bugs.launchpad.net/bugs/364168 . 
Comment 10 radu tanasescu 2009-06-13 14:39:06 UTC
(In reply to comment #9)
> Radu, the problem is fixed in the latest version of Rhythmbox. For Ubuntu see
> https://bugs.launchpad.net/bugs/364168 . 
> 

Hi Hew,

Thank you for your replay.
I just installed the newest version (karma) and the drag and drop downscrolling is now available and works fine.
Nonetheless, by quickly passing from a track to another (doubleclicking), Rhythmbox gets blocked for a while and sometimes I have to kill it.
I guess this is due to the fact that my system is a Jaunty64 while the version is still a "work in progress for Karma" one.

Thank you anyway for your kindness.

All the best,
radu