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 484047 - gtk_notebook_remove_page() ignores the specified page_num
gtk_notebook_remove_page() ignores the specified page_num
Status: RESOLVED NOTABUG
Product: gtk+
Classification: Platform
Component: Widget: GtkNotebook
2.10.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2007-10-06 09:37 UTC by Koichi Momma
Modified: 2017-08-15 16:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Testcase for gtk_notebook_remove_page (1.31 KB, text/plain)
2007-10-07 09:19 UTC, Xan Lopez
Details
Example code to reproduce the problem (1.42 KB, text/plain)
2007-11-17 10:26 UTC, Koichi Momma
Details

Description Koichi Momma 2007-10-06 09:37:40 UTC
gtk_notebook_remove_page() always removes the first tab in the notebook regardless of the page_num index. This bug was introduce by the changes in 2.9.x development cycle.

A possible patch for GTK+ 2.10.13 to fix this bug is as follows:


--- gtknotebook.c.orig	2007-06-13 12:09:42.000000000 +0900
+++ gtknotebook.c	2007-07-11 19:46:20.000000000 +0900
@@ -6167,8 +6167,8 @@
     list = g_list_last (notebook->children);
 
   if (list)
-    gtk_container_remove (GTK_CONTAINER (notebook),
-			  ((GtkNotebookPage *) list->data)->child);
+    gtk_notebook_real_remove(notebook, list);
+
 }
 
 /* Public GtkNotebook Page Switch Methods :
Comment 1 Xan Lopez 2007-10-07 09:19:08 UTC
Created attachment 96804 [details]
Testcase for gtk_notebook_remove_page

Not true? The obvious testcase seems to work fine. Also, reading the code I'd say it does the right thing. What are you trying to do exactly?
Comment 2 Koichi Momma 2007-10-07 16:23:32 UTC
I understand that the test case works correctly.
I'm using gtk from wxGTK so I am sorry if I bother you with wrong report.

This bug occurs when I use wxGTK with gtk 2.10.x or above and the only difference between gtk 2.8.x and 2.10.x that may cause the problem is this function.

I thought this is gtk bug because the above patch fixes the problem. But maybe I should've reported it to wxWidgets' list first.

Thank you.
Comment 3 Xan Lopez 2007-10-07 16:48:12 UTC
I'm not sure why that patch fixes your problems, but at any rate it's wrong. There's code in gtk_notebook_remove (what ends up being called when you do a gtk_container_remove) that needs to be executed, like the emission of the page-removed signal. The code is pretty wasteful as it seems to be doing:

remove_page:
 find the nth item in the children list, get its data and pass it to remove.

container_remove:
 find the list item with the data element and pass it to real_remove.

container_real_remove:
 finally remove the element.

Maybe emitting the signal from page_remove and calling real_remove from there would be better?
Comment 4 Koichi Momma 2007-10-12 09:23:41 UTC
I am not sure about how this function should be. I am still searching the reason for my problem.

So if current implementation is not very sophisticated, please make it better. Otherwise leave it as it is.

If I can find that gtk+ implementation is really responsible for my problem, I will ask for changes again.
Comment 5 Koichi Momma 2007-11-17 10:26:33 UTC
Created attachment 99249 [details]
Example code to reproduce the problem

This problem occurs when reusing the same widgets in different notebook pages. The attached example works correctly on gtk+ 2.8.x, whereas it won't on 2.10.x or above.

In the previous implementation, gtk_notebook_remove_page() directly remove the specified page. From gtk 2.10.x on, gtk_notebook_remove() will be called by gtk_container_remove(). However, gtk_notebook_remove() does not receive the 
nth information. It only receives pointer of the child widget to be removed and searches the page, by 

if (page->child == widget).

When reusing the same widget in the all notebook pages, the above condition will always be satisfied, and ends up the first tab to be removed in any case.

Emitting the signal from gtk_notebook_remove_page and directly calling real_remove from there would solve this problem.

Reusing the same widgets may not be normal usage of gtk+ but at least it was possible before, and I hope to do so in newer version of gtk+.
Comment 6 Luca Bruno 2007-11-29 11:17:37 UTC
Hello,
these new changes are, in my mind, right.
If you call real_remove from remove_page, you can't emit the container remove signal because will remove another page.

Think other widgets having the same children, wouldn't be good. The same now is with notebooks, it's not clean to have the same widget forced to be multiple times child of the same parent.

However, do not follow my statements, i'm new here.
Comment 7 Koichi Momma 2007-11-30 13:06:46 UTC
Thank you for the answer.

> Think other widgets having the same children, wouldn't be good.
I know, and still I wanted to do that. Think about a canvas widget placed in notebook pages. Since notebook widget ensures that more than two canvases will never be visible at the same time, we don't actually need multiple canvases. Reusing single canvas will save memory resources. If there is a way to display "tabs" on top of single canvas, it is the better rather than adding the same canvas to multiple notebook pages.

So even if reuse of widgets is not recommended, I hoped that gtk+ won't inhibit it.
Unfortunately, there seems to be a good reason to inhibit such usage.

Anyway, thank you again.
Comment 8 Daniel Boles 2017-08-15 16:10:17 UTC
The function does what it should do.

It breaking when you add the same widget to multiple pages just reflects the broken code: a widget cannot be added to more than one container.

That is not going to change. To defend your use of it by talking only about drawing is a huge oversimplification that ignores the role of containers in managing the lifetime of their children: think double-frees, use-after-frees, and all kinds of other horrors.


(In reply to Koichi Momma from comment #5)
> Reusing the same widgets may not be normal usage of gtk+ but at least it was
> possible before, and I hope to do so in newer version of gtk+.

I'm afraid this isn't an argument. You were writing invalid code and relying on its undefined behaviour of _happening to work_ in the past. That's not an argument that it should work that way now; it's evidence that the code was invalid and the design needs to be restarted.


(In reply to Koichi Momma from comment #7)
> Think about a canvas widget placed in
> notebook pages. Since notebook widget ensures that more than two canvases
> will never be visible at the same time, we don't actually need multiple
> canvases. Reusing single canvas will save memory resources.

Of course, wanting to be efficient is an admirable goal. But using a widget in multiple locations at once is not the way to achieve it.

The proper way to handle this would be to detect change of tab and remove/add the widget as required. So you can still save resources and whatever, but you're not going completely against the models of widget hierarchy and memory management that GTK+ relies on. I hope this helps?