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 667590 - weird code in scale: find_next_pos()
weird code in scale: find_next_pos()
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.24.x
Other Linux
: Normal trivial
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2012-01-09 17:41 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2012-01-20 20:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for gtk2 (3.06 KB, patch)
2012-01-09 17:56 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review
patch for gtk3 (3.32 KB, patch)
2012-01-09 18:12 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
needs-work Details | Review
patch for gtk3 (2.65 KB, patch)
2012-01-10 13:14 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2012-01-09 17:41:40 UTC
I wonder why in find_next_pos() the code passes the match param, in gtk2 we always pass position=LEFT/TOP and match=0/1, in gtk3 we pass position=LEFT,RIGHT/TOP,BOTOM and match is always=1, I would just drop the match param. It works just fine without and passing the proper position value. Will attach patches shortly.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2012-01-09 17:56:30 UTC
Created attachment 204895 [details] [review]
patch for gtk2
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2012-01-09 18:12:47 UTC
Created attachment 204898 [details] [review]
patch for gtk3
Comment 3 Matthias Clasen 2012-01-10 00:09:58 UTC
Review of attachment 204898 [details] [review]:

Removing the redundant match argument is fine, the other change not.

::: gtk/gtkscale.c
@@ +1180,3 @@
           else
             {
+              if (mark->position == GTK_POS_LEFT)

I'm sure that that this is not correct.
See the comment in line 100, and the code in gtk_scale_add_mark.
Comment 4 Matthias Clasen 2012-01-10 00:11:48 UTC
Review of attachment 204898 [details] [review]:

Removing the redundant match argument is fine, the other change not.

::: gtk/gtkscale.c
@@ +1180,3 @@
           else
             {
+              if (mark->position == GTK_POS_LEFT)

I'm sure that that this is not correct.
See the comment in line 100, and the code in gtk_scale_add_mark.
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2012-01-10 13:14:04 UTC
Created attachment 204940 [details] [review]
patch for gtk3

Oh that LEFT is TOP and BOTTOM is RIGHT was too clever for me. Honestly I think the gtk2 code was better for this. The LEFT/TOP also helps to quickly see whether one change the horizontal or vertical part of the code.
Comment 6 Matthias Clasen 2012-01-15 19:17:57 UTC
I've removed the match parameter now
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2012-01-16 10:07:27 UTC
could you please also push the changes to 2.24?