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 658694 - GtkAssistant: Unable to change current page in prepare handler
GtkAssistant: Unable to change current page in prepare handler
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.1.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2011-09-10 06:48 UTC by Sébastien Granjoux
Modified: 2012-09-17 03:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix (922 bytes, patch)
2011-10-15 08:35 UTC, Sébastien Granjoux
needs-work Details | Review
Another fix (922 bytes, patch)
2012-09-08 12:44 UTC, Sébastien Granjoux
reviewed Details | Review
Fix checking page number (954 bytes, patch)
2012-09-09 21:00 UTC, Sébastien Granjoux
accepted-commit_now Details | Review

Description Sébastien Granjoux 2011-09-10 06:48:27 UTC
Anjuta uses the GtkAssistant widget and creates page dynamically. The order of the pages is changing while the assistant is running. Anjuta uses the gtk_assistant_set_current_page function.

A common usage is to have a dummy page after the current one. Then when the user clicks on "Next", the "prepare" handler of this dummy page is triggered and is responsible for finding the real following page and going to it using gtk_assistant_set_current_page.

This usage is broken in the latest version of Gtk. The offending code is here:

static void
set_current_page (GtkAssistant *assistant,
                  gint          page_num)
{
   GtkAssistantPrivate *priv = assistant->priv;

priv->current_page = (GtkAssistantPage *)g_list_nth_data (priv->pages, page_num); g_signal_emit (assistant, signals [PREPARE], 0, priv->current_page->page);

   update_title_state (assistant);
   gtk_notebook_set_current_page (GTK_NOTEBOOK (priv->content), page_num);

After emitting the "prepare" signal, the note book page is still set with the old page number. So it cannot be changed in the prepare handler.
Comment 1 Matthias Clasen 2011-09-15 12:55:49 UTC
Why don't you use the intended mechanism for this kind of dynamic page loading ?
gtk_assistant_set_forward_page_func
Comment 2 Sébastien Granjoux 2011-09-15 19:10:28 UTC
(In reply to comment #1)
> Why don't you use the intended mechanism for this kind of dynamic page loading
> ?
> gtk_assistant_set_forward_page_func

I already use this function but it's not what I need. 

This function is not called when the user click on the next button. It is called as soon as a new page is displayed to know what is the next page. I think the main use is rather to know if such page exist to disable or not the Next button. In my wizard, at that time, I don't know what will be the next page. I know this only after the user has filled the current page and clicks on next.

In fact, I use gtk_assistant_set_forward_page_func to insert a dummy page after each normal page and I use the "prepare" handler of this dummy page to switch to the right page.


To be really complete, the next page is generated by calling an external program based on the information just filled by the user on the current page. So in the common case, I don't change the page in the "prepare" handler. But this is an issue if the user inputs are invalid or there is an error when starting the program because in this case I know in the prepare handler what is the next page.
Comment 3 Sébastien Granjoux 2011-10-15 08:35:46 UTC
Created attachment 199054 [details] [review]
Fix

Here is a fix for this bug. After returning from the prepare handler, I have updated page_num to be sure that it matches the current page and set the right page in the GtkNotebook.
Comment 4 Sébastien Granjoux 2012-02-04 09:12:07 UTC
I'm still interested to get this fix in gtk+. Is there someone available to review the patch?
Comment 5 Cosimo Cecchi 2012-09-03 16:12:01 UTC
Review of attachment 199054 [details] [review]:

I'll try to review this:

::: gtk/gtkassistant.c
@@ +719,3 @@
+  page_num = g_list_index (priv->pages, priv->current_page);
+  if (page_num == -1)
+    return;

Since this is the only function that updates the current page, I think I'd rather avoid running the state update code at all if the prepare handler somehow changed it again, like

if (page_num_after_prepare != page_num)
  return;

If I understood the bug correctly, it should still fix your issue.
Comment 6 Sébastien Granjoux 2012-09-03 21:01:58 UTC
Thanks for the review

(In reply to comment #5)
> if (page_num_after_prepare != page_num)
>   return;
> If I understood the bug correctly, it should still fix your issue.

Yes, I think it will be fine.

I have removed the work around in my code to check but it is quite different due to other changes in GtkAssistant and I don't see the issue anymore. Anyway, I think it's still useful to fix it.
Comment 7 Sébastien Granjoux 2012-09-08 12:44:41 UTC
Created attachment 223809 [details] [review]
Another fix

Here is a patch that do not run the state update code if the prepare handler has already changed it.

I have tried in Anjuta and it fixes the issue.
Comment 8 Cosimo Cecchi 2012-09-09 20:45:28 UTC
Review of attachment 223809 [details] [review]:

::: gtk/gtkassistant.c
@@ +719,3 @@
+  page_num = g_list_index (priv->pages, priv->current_page);
+  if (page_num == -1)
+    return;

Why do you check for page_num == -1 instead of it being != from the previous value? It looks to me that the prepare handler could also change the current page to an existing one, and this would break that case.
Comment 9 Sébastien Granjoux 2012-09-09 21:00:34 UTC
Created attachment 223863 [details] [review]
Fix checking page number

Sorry, I have uploaded the previous patch again, here is the right one.
Comment 10 Sébastien Granjoux 2012-09-09 21:05:30 UTC
(In reply to comment #8)
> Why do you check for page_num == -1 instead of it being != from the previous
> value? It looks to me that the prepare handler could also change the current
> page to an existing one, and this would break that case.

Anyway, I think this patch is working because if the prepare handler has changed the page number, priv->current_page should be updated too.

But it's true that you rerun the set current page which shouldn't be necessary.
Comment 11 Cosimo Cecchi 2012-09-09 21:08:38 UTC
Review of attachment 223863 [details] [review]:

Thanks, this looks good to me!