Hi,
Lillian Angel wrote:
This patch fixes the long standing problem of not being able to
dynamically switch between a checkbox and a radio button. I.e. moving a
checkbox to a checkbox group changes the checkbox to a radio button.
Tom helped a lot with this patch. He removed the CheckboxGroupPeer and
we fixed it so everything is handled in GtkCheckboxPeer.
There is a mauve test for this. The harmony test
(test.java.awt.CheckboxTest) now passes.
This looks good, but I have a few comments:
+/* A radio button is created if we are part of a group.
+ groupPointer points to the corresponding group. If 0,
+ a new group is created.
+ This function is called when initially creating the
+ button, so an eventbox is created.
+ */
+JNIEXPORT jlong JNICALL
+Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_createRadioButton
+ (JNIEnv *env, jobject obj, jlong groupPointer)
+{
+ GtkWidget *button;
+ GtkWidget *eventbox;
+ GSList *native_group = NULL;
+
+ gdk_threads_enter ();
+
+ NSA_SET_GLOBAL_REF (env, obj);
+ eventbox = gtk_event_box_new ();
+
+ if (groupPointer != 0)
+ {
+ native_group = JLONG_TO_PTR (GSList, groupPointer);
+ if(! GTK_IS_RADIO_BUTTON (native_group->data))
+ native_group = NULL;
When does it happen that the data field is not a GtkRadioButton? I suspect only
after removing a Checkbox from a CheckboxGroup? This is likely a memory leak --
if the native_group's data field is no longer valid then we should probably free
the GSList.
+ }
+ button = gtk_radio_button_new_with_label (native_group, "");
+
+ if (native_group == NULL)
+ native_group = gtk_radio_button_get_group (GTK_RADIO_BUTTON (button));
+ if (g_slist_index (native_group, GTK_RADIO_BUTTON (button)) == -1)
+ {
+ native_group = g_slist_prepend (native_group, GTK_RADIO_BUTTON (button));
+ GTK_RADIO_BUTTON(button)->group = native_group;
+ }
+
+ gtk_container_add (GTK_CONTAINER (eventbox), button);
+ gtk_widget_show (button);
+
+ NSA_SET_PTR (env, obj, eventbox);
+
+ gdk_threads_leave ();
+
+ return PTR_TO_JLONG (native_group);
I realized that there is a race condition here. Instead of returning the
pointer value, we should use JNI to set the groupPointer field with the GDK lock
held.
+}
+
+/* Add the object to the group pointed to by groupPointer.
+ If groupPointer is 0, create a new group and create
+ a radio button. Otherwise, creating a radio button in an
+ existing group.
+ */
+JNIEXPORT jlong JNICALL
+Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_addToGroup
+ (JNIEnv *env, jobject obj, jlong groupPointer)
+{
+ void *ptr;
+ GtkWidget *container;
+ GtkWidget *check_button;
+ GtkWidget *radio_button;
+ const gchar *label;
+ GSList *native_group = NULL;
+
+ gdk_threads_enter ();
+
+ ptr = NSA_GET_PTR (env, obj);
+ container = GTK_WIDGET (ptr);
+ check_button = checkbox_get_widget (container);
+ label = gtk_label_get_text (GTK_LABEL (gtk_bin_get_child
+ (GTK_BIN (check_button))));
+
+ /* Need to remove the check_button, and replace it with
+ a radio button in a group.
+ */
+ if (groupPointer != 0)
+ {
+ native_group = JLONG_TO_PTR (GSList, groupPointer);
+ if(! GTK_IS_RADIO_BUTTON (native_group->data))
+ native_group = NULL;
+ }
+
+ radio_button = gtk_radio_button_new_with_label (native_group, label);
+ gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (radio_button),
+ gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (check_button)));
+
+ if (native_group == NULL)
+ native_group = gtk_radio_button_get_group (GTK_RADIO_BUTTON
(radio_button));
+ if (g_slist_index (native_group, GTK_RADIO_BUTTON (radio_button)) == -1)
+ {
+ native_group = g_slist_prepend (native_group, GTK_RADIO_BUTTON
(radio_button));
+ GTK_RADIO_BUTTON(radio_button)->group = native_group;
+ }
+
+ gtk_container_remove (GTK_CONTAINER (container), check_button);
+
+ NSA_DEL_PTR (env, obj);
+ NSA_SET_PTR (env, obj, container);
The NSA table points to the containing event box, so I don't think these two
lines are necessary.
Tom
