GNOME Bugzilla – Bug 700914
Add GtkHeaderBar support
Last modified: 2014-11-07 10:21:39 UTC
So I can create composite widgets from it.
Created attachment 250961 [details] [review] Add GtkHeaderBar support
Works for simple use cases Missing * an icon * multiple start/end children support * custom-title widget support Will fix coding style when I get back from GUADEC
Hmm, this indeed shows the GtkHeaderBar for me in glade, but it also doesn't show the GtkWindow contents, i.e. the main bit in the UI. It *just* shows the header and nothing else.
(glade:16629): Gtk-WARNING **: Attempting to add a widget with type GtkBox to a GtkWindow, but as a GtkBin subclass a GtkWindow can only contain one widget at a time; it already contains a widget of type GtkHeaderBar
That warning is because glade does not recognize <child type="titlebar">...</child>. If you place the GtkHeaderBar within that, you can add the main window UI in the <child>...</child> element and it will work in the application and glade-previewer. Glade will remove the |type="titlebar"| from a child node.
Created attachment 275189 [details] [review] Initial GtkHeaderBar support Add support for GtkHeaderBar. Based on a patch by John Stowers.
Slight elaboration of Johns patch. It adds icons, and allows to add/remove slots. Still missing: - support for replacing title+subtitle with a custom widget - start/end size pseudo properties - support for the 'titlebar' slot on windows
Created attachment 275191 [details] [review] Support CSD windows This patch adds preliminary support for the titlebar slot in GtkWindow. The rendering of windows with csd titlebar is somewhat broken in glade, and the virtual use-csd property does not get set up properly when loading a ui file.
I tested Matthias' 2 patches on top of glade master. Opening gnome-control-center/panels/sharing/sharing.ui says "Property has versioning problems: Use Header Bar". Toggling the "CSD" switch twice will hide all the contents of the window. Seems that the patch needs more work for the "use_header_bar" GtkDialogs.
The window content disappearing can be "fixed" by putting a gtk_widget_map (GTK_WIDGET (object)); after the gtk_window_set_titlebar (GTK_WINDOW (object), NULL); Though GTK+ will still complain you set a titlebar on a mapped window.
I've pushed a headerbar branch, which works well enough for me. Supports custom titles, and multiple children at either end.
(In reply to comment #11) > I've pushed a headerbar branch, which works well enough for me. Supports custom > titles, and multiple children at either end. So I tried the branch and have some preliminary comments before really digging into the patches themselves. First problem I found, The CSD property can only be set to TRUE once, after which point setting the property has no effect and there are errors in the console about calling gtk_window_set_titlebar() on a realized window. I'm not sure why this works the first time, actually, it's possible that we need that virtual property to behave like construct-only properties but as I'm not sure how it could work on an already displayed window the first time, I'm really unsure. Setting the CSD property to TRUE and doing Undo/Redo has the same effect (top placeholder is lost and can never be restored). These patches (probably the first one) break GtkWindow, it no longer offers a placeholder for the actual window content. The header bar properties have some properties which seem irrelevant for a project using Glade, i.e. the 'Has Subtitle' and 'Decoration Layout Set' We should consider either: a.) Following the pattern in Glade's custom editors where there is a checkbox for the 'Has Subtitle' and when that is disabled, the text entry for the subtitle text on the right is disabled. This is only a good idea if it's really wanted to have a headerbar with no subtitle but still 'reserving space for the subtitle'. b.) Probably more appropriately, we just hide these properties altogether from the UI and sync them up in the plugin in various places: - At load time, set has-subtitle according to whether subtitle text is actually provided - At save time, set do the same - For the runtime appearance, adjust the runtime widget's has-subtitle whenever the subtitle text changes Selecting a headerbar child and modifying it's GtkPackType does not effect the placement of a child widget at runtime, however if you save it and load it then the child appears on the other side. The "Add slot at start" / "Add slot at end" actions are only set as child actions, but should probably also be added as widget actions of the headerbar type proper (i.e. right clicking on a child on the left or right shows the action, right clicking on the headerbar itself does not let you add/remove slots). Furthermore, the add/remove slot actions effect project data without going through the glade command API (so adding/removing slots is not undoable and can cause errors in large undo/redo chains). Further, the add/remove slot actions do not result in changing of the virtual properties "Number of items at the start" and "Number of items at the end" Quick pointer: Placeholders are not project data but generated by the adaptors based on thier own design, most bin widgets always ensure a placeholder when no project widget is there, things like GtkBox/GtkGrid ensure that placeholders exist by dynamically generating them whenever relevant properties change and when widgets are added/removed. So basically program flows as: o User interacts with Glade o Glade issues a command o The command effects a Glade property or add/remove widget o The adaptor ensures that placeholders exist Some other details, the CSD property should declare a <_tooltip> as it's meant to be visible in the UI and there is no other tooltip source (virtual properties that show up in the UI should all declare an expanitory tooltip). Glossing over the patch I've found glade_command_push_group() called without a real command description, instead only a widget name is used. Command descriptions should be brief but explanitory, i.e. "Creating $foo" or "Setting dimentions of $foo", etc.
A few more notes on this: Setting the CSD property to TRUE / FALSE does not delete the children which might already exist, or restore them at undo time There are two ways to do this proplerly: o The preferred way to do this is with a custom editor, like most widgets already have. In a custom editor, one could add a GtkCheckButton directly for the CSD property and issue GladeCommands when that button state changes. o There is a signal 'pre-commit' and 'post-commit' on the GladeEditorProperty object which actually displays the property in the editor. These signals can be caught to trap the action into a command group, allowing one to issue GladeCommands before and after, one could use this approach to delete the titlebar content undoably For the record, in my last comments I mentioned the awkwardness of the headerbar properties 'has-subtitle' and 'decoration-layout-set', I do not consider these issues to particularly block the initial landing of this branch. It will probably be a high priority before rolling the next stable Glade though, because if we restrict users to not expose the 'has-subtitle' property, it's probably best to do it from day one, and not remove it in a later release (users can set that property manually if that's needed for dynamically set subtitles, but it will break their apps if allow them to set it in Glade and then suddenly remove it later on). Thanks for your serious attention to this Matthias, headerbar presents a lot of work, I'll do my best to keep up with reviews in the limited time I have.
toggling the setting works fine here. yes, you get a warning about the realized window, but we do the right thing there.
(In reply to comment #14) > toggling the setting works fine here. yes, you get a warning about the realized > window, but we do the right thing there. This is a capture taken right now: https://people.gnome.org/~tvb/test.ogv It's running against a GTK+3 3.13 from the looks of my latest GTK+ configure.ac, with an x11 backend.
Rebuilt GTK+3 master just to re-confirm, same problem persists.
I'll look again. Toggling csd on and off like that is not really something that ever happens in real life, so it is poorly tested. Maybe an alternative to the switch would be a separate 'csd window' object - can glade do 'fake types' like that ?
https://git.gnome.org/browse/gtk+/commit/?id=8821d488c52ca0d571fc76c8160f181c6aa43f41 fixes the content visibility problem when repeatedly toggling the csd switch I've also moved the use-csd property into the GladeWindowEditor. I think the one blocker thats left before I consider this good enough is a way to remove an existing headerbar widget when csd is toggled back off. It disappears from the preview, but it still hangs around in the widget tree. I just don't know enough glade internals to figure out how to make it go away.
(In reply to comment #18) > https://git.gnome.org/browse/gtk+/commit/?id=8821d488c52ca0d571fc76c8160f181c6aa43f41 > > fixes the content visibility problem when repeatedly toggling the csd switch > > I've also moved the use-csd property into the GladeWindowEditor. I think the > one blocker thats left before I consider this good enough is a way to remove an > existing headerbar widget when csd is toggled back off. It disappears from the > preview, but it still hangs around in the widget tree. I just don't know enough > glade internals to figure out how to make it go away. Hi Matthias, I've pushed a commit onto your branch (and reverted the commit which issues glade commands from the GladeWidgetAdaptor->set_property() method) here: https://git.gnome.org/browse/glade/commit/?h=headerbar&id=32fc32041c333790b7b87ba0e88831865836d820 It can be confusing since there are various levels at which one can participate in how Glade handles widgets, hopefully the above commit clarifies this a bit (Note specifically that the GladeWindowEditor includes a check button, and when the check button changes state, the editor pushes a command group which changes the CSD setting and possibly deletes a project widget), this makes the whole thing properly undoable and in one command group. Basically: o The GladeWidgetAdaptor->set_property()/get_property() methods are called whenever a widget should be updated in Glade's workspace. Objects in the workspace are updated whenever GladeWidget properties change, which might be due to some user input, and might also be due to an Undo/Redo command. So it's important to not issue new commands at this point ;-) o Editors issues commands, those are both widget-level editors and property level editors. When the user interacts with an editor, it should always be GladeCommand API that is used to effect the data model. GladeWidgetAdaptor offers methods for the creation of editor classes (both property editors and widget editors). Anyway, the fix you pushed to GTK+ indeed fixes the re-appearance of the titlebar, and the commit I pushed to your branch makes the CSD property properly undoable and consequentially deleted project widgets reappear at undo time. This took me a chunk of my day so I don't really have time for another full review, please take a look at the headerbar branch and let me know when you think the branch is ready for another review.
Actually while I'm here though... I think that at least the client side decorations feature is at least just about ready to land. If you can split out the 'use-csd' property related patches and merge only those to master, then we can concentrate on the headerbar widget in this bug. There are still 2 concerns I still have regarding the 'use-csd' patches: o I think that the use-csd setting has to be exclusive to some other properties, however I'm not sure which ones they are really. For instance, the 'window title' property is still meaningful with csd enabled ? I would suspect it is still useful even if it doesnt show up in the titlebar (i.e. it's still a word that will show up in some window managers task lists correct ?) Are the 'decorated' and 'hide titlebar during maximization' properties meaningful if the application provides a custom titlebar ? o The static function you added searches the window hierachy for a child widget with the style class 'headerbar', this feels error prone, like it may break if a user ever adds a widget somewhere with the 'titlebar' class, or that it may break with an upgrade to a future GTK+ version. Can we please have something more stable to retrieve the titlebar ? Perhaps an API in GTK+ proper for this ? In any case, I still think we can safely land the CSD portions of this branch and address the above issues separately.
(In reply to comment #19) > This took me a chunk of my day so I don't really have time for another full > review, please take a look at the headerbar branch and let me know when you > think the branch is ready for another review. Thanks, Tristan. I'll check it out.
(In reply to comment #20) > There are still 2 concerns I still have regarding the 'use-csd' patches: > > o I think that the use-csd setting has to be exclusive to some other > properties, however I'm not sure which ones they are really. > > For instance, the 'window title' property is still meaningful with > csd enabled ? I would suspect it is still useful even if it doesnt > show up in the titlebar (i.e. it's still a word that will show up > in some window managers task lists correct ?) > > Are the 'decorated' and 'hide titlebar during maximization' properties > meaningful if the application provides a custom titlebar ? You are right, hide-titlebar-when-maximized and decorated ignored for csd windows, we should probably reflect that in the UI. The title is passed on to the headerbar (if you use a GtkHeaderBar for the titlebar widget). But GtkHeaderBar exposes the title itself, so we could just as well make make the title property go away for csd windows. > o The static function you added searches the window hierachy for a child > widget with the style class 'headerbar', this feels error prone, like > it may break if a user ever adds a widget somewhere with the 'titlebar' > class, or that it may break with an upgrade to a future GTK+ version. > > Can we please have something more stable to retrieve the titlebar ? > > Perhaps an API in GTK+ proper for this ? I've been thinking the same.
The title propagation doesn't quite work in glade though - glade doesn't pick up on the fact that the headerbar title changes 'spontaneously'.
I've pushed two new branches: csd - containing the cleaned up csd support, including your editor fixes. The only changes on top of what was in the original headerbar branch is that I make the title, decorated and hide-when-maximized controls insensitive when csd is on, and I use gtk_window_get_titlebar from GTK+ when available. headerbar - a new headerbar branch, containing just the commit that adds support for GtkHeaderBar as a widget.
(In reply to comment #24) > I've pushed two new branches: > > csd - containing the cleaned up csd support, including your editor fixes. The > only changes on top of what was in the original headerbar branch is that I make > the title, decorated and hide-when-maximized controls insensitive when csd is > on, and I use gtk_window_get_titlebar from GTK+ when available. Hi, thanks for splitting it out. Sorry to bring up another 'gotcha', but the way you made those properties appear insensitive is broken, and we have facilities to do that properly. Broken issues with insensitivity: o When loading a project with a window that has CSD turned on, the properties are not insensitive o When there is more than one window, one with CSD on and another not, selecting one and then the other shows that insensitive state is not synced to project state o When turning on CSD and doing Undo/Redo, sensitivity does not follow No worry, you didn't know, just highlighting the reasons of why we have a facility for this, which is glade_widget_property_set_sensitive(), it needs to be called from the GladeWidgetAdaptor->set_property() method whenever the "CSD" property changes state. Here is an example of how we control sensitivity based on GtkImage's virtual "edit-mode" property (which shows up as a radio button in the UI): https://git.gnome.org/browse/glade/tree/plugins/gtk+/glade-gtk-image.c#n87 As an added bonus, the API also sets the tooltip which will be shown when the property is insensitive (so we can have that say something like "This property has no effect while CSD is enabled"). Note, what I also normally do when one property causes another one to be insensitive, is I also set it to the default value in the issuing GladeCommand group. Another GtkImage example: https://git.gnome.org/browse/glade/tree/plugins/gtk+/glade-image-editor.c#n138 This should also be done for the 'decorated', 'title' and 'hide-when-maximized' properties from the same command group which possibly deletes the titlebar child, in glade-window-editor.c So, to sum it up: o Please move the gtk_widget_set_sensitive() calls out of glade-window-editor.c and add calls to glade_widget_property_set_sensitive() to glade-gtk-window.c in the GladeWidgetAdaptor.set_property() method o Please set those property to their defaults undoably from inside the GladeCommand group, *only* when the CSD property is becomming TRUE. And then we can go ahead and merge the branch.
> So, to sum it up: I hope I've done these three things now, pushed a new csd branch with an additional commit.
(In reply to comment #26) > > So, to sum it up: > > I hope I've done these three things now, pushed a new csd branch with an > additional commit. Great, everything in the csd branch is working great. I merged it to master now so we can move on. Also, I made 2 changes on top of your branch: o #define CSD_DISABLED_MESSAGE _("This property does not apply...") I'm not certain if it's really needed, but it's a practice we've been doing for a long time, i.e. avoiding redundant translatable strings (we just ensure the same string doesnt show up multiple times in the po files). o Just made master depend on GTK+ 3.15.0 and removed the GTK_CHECK_VERSION ifdefs around gtk_window_get_titlebar().
Comment on attachment 250961 [details] [review] Add GtkHeaderBar support Marking patch as obsolete, headerbar work is currently under review in the headerbar branch
Comment on attachment 275189 [details] [review] Initial GtkHeaderBar support Marking patch obsolete, headerbar work is taking place on the headerbar branch.
Comment on attachment 275191 [details] [review] Support CSD windows Marking patch as obsolete, csd window support was further improved under the 'csd' branch and now merged in master.
Matthias, I took a look at headerbar today and see it's still in the same state it was in (nothing headerbar related has been addressed since comment 12). Anyway, here is a list of issues I have with headerbar: o What is the user expected to put in "decoration-layout" ? Looking at gtk_header_bar_set_decoration_layout(), this looks like a rather cumbersome API to expose in Glade, so until we come up with a good UI for this, let's just assume the user is expected to set a string. o What about the "decoration-layout-set" property of headerbar ? Is this really a property which should be programatically set ? I think it's an implementation detail and should be omitted from Glade. o Similar story of 'has-subtitle' I don't think Glade should allow the user to set the 'has-subtitle' property. It it important that has-subtitle should be set for a user's subtitle to appear ? It's not that clear from a quick read of gtkheaderbar.c If it's important to set has-subtitle, then we should hide the has-subtitle property in Glade and set it to TRUE automatically at GladeWidgetAdaptor->write_widget() time. If it's not important that the user sets this property, it should be disabled in Glade. o More orphaned widget issues with the "Custom Title" property. This will unfortunately require a custom editor to do properly (hopefully that's not too difficult as there are plenty of examples to start from). Set "Custom Title" to TRUE, then add a widget in the placeholder at the center of the headerbar, then set "Custom Title" to FALSE: --> Notice the added child still shows up in the widget tree even though the custom title widget was removed, it's still in the project. Now Hit Undo: --> Notice the child which was added does not re-appear properly at undo time. o Also related to the custom-title property, when the user sets the custom title to TRUE, this should undoably clear the text which may have previously been set in the "title" property. o While custom-title is TRUE, we should have the "title" property insensitive with an informative message telling the user that the title property does not apply when a custom title is set.
thanks for the list. I'll look for some time this weekend to work through it.
I pushed a new headerbar branch that should address your concerns by adding a headerbar editor.
Note that you need 287319564450211a88e313d99eb3381016ae8074 in gtk+ to avoid a segfault when resurrecting a custom title child via undo. One thing I couldn't quite figure out is how to make the entire decoration-layout row insensitive when show-decorations is unset. Not really a big deal though.
Good progress, now that I play with it in depth I'm finding some rougher areas. o Placeholder disappears on Undo/Redo Steps: a.) Create a headerbar (no need for a window or anything, can be in a window or not, doesnt make a difference) b.) Set the 'Custom Title' property to TRUE c.) Add a button to the left hand placeholder in the headerbar d.) Drag'n'Drop the button from the left position to the center placeholder e.) Undo f.) Redo Now notice that the left placeholder has disappeared completely In the console is printed the message: Gtk-WARNING **: Can't set a parent on widget which has a parent This points to a probable error when the adaptor is called with replace_child() or add/remove_child() with an existing placeholder from the undo/redo code (GladeCommand does not use the GtkContainer API directly). Note that the message: Gtk-WARNING **: gtkcontainer.c:1289: child property `position' of container class `GtkHeaderBar' is not writable Is issued any time something is positioned in the headerbar, which may indicate that the adaptor code does not have the right expectations. o Undo/Redo Crashes This issue occurs with the same steps above... now that you have the missing placeholder, continue from step f: g.) Undo until the project is empty (until before headerbar creation) h.) Redo until you get back to the end. Now we get a crash, and on the console we see these messages: GladeUI-WARNING **: Discrepancy found in TreeModel data proxy. Can not get children iter for widget headerbar1 GladeUI-WARNING **: Internal data model error, object 0x38cc530 GtkButton not found in tree model Gtk:ERROR:gtkcontainer.c:3656:gtk_container_propagate_draw: assertion failed: (gtk_widget_get_parent (child) == GTK_WIDGET (container)) o Child action for 'Add Slot at start' and 'Add Slot at end' create a command group but do not seem to effect project data (and the command group does not even have a proper title) The way we normally do this for box is we manage the 'size' virtual property and the position properties of children (which *are* project data), then, as a consequence of project data changing, when the "size" property changes, the adaptor ensures there are enough placeholders. o When adding a slot at start/end, the "Number of items at start" and "Number of items at end" properties are not properly updated o "Number of items at start" property can be decreased to 0 even if a project widget is added there, oddly, decreasing the number of items to 0 does not even cause the added widgets to disappear or orphan any project data o Copy/Paste does not preserve the number of placeholders on the left and right sides of the headerbar a.) Create headerbar b.) Set the "number of items at start" to 2 c.) Copy the headerbar d.) Create an arbitrary widget or window with placeholders d.) Paste the headerbar to the new parent Result: The copied headerbar only has 1 placeholder at the start, not 2 Observations: This only happens with the placeholders at the start, it seems that placeholders at the end are handled alright for some reason. Regarding the above four which all have to do with the virtual "start-size" and "end-size" properties, they need to be implemented properly, i.e.: - User issued commands need to effect the actual project data (i.e. the headerbar properties). - Adaptor must respond to property changes and ensure the existence of placeholders accordingly - The verify_child_property() vfunc needs to be implemented to ensure that the number cannot decrease to an amount which results in orphaned project widgets Let's continue: o The 'Add slot at start' / 'Add slot at end' child actions are useful, they should also at least be added as widget actions There are 2 brands of actions, those that appear in a context menu when you click on a child, and those that appear when you click on the actual widget, this action is more appropriate for the base widget action, but could be useful in for both, currently its only declared as a 'child' action. o Some coding style issues. In glade we do not declare functions with trailing '{' and we always put the opening curly brace on a new line, not at the end of an if statement Also, we usually: call_function (WITH_ARG (one), ARG (two)); Not call_function( WITH_ARG(one), ARG(two) ); Code style seems to suffer from C++itis ;-) o To answer your question about making the decoration-layout properties insensitive, it is done in the normal way from the adaptor code using the glade_widget_property_set_sensitive() functions (as we did already in another one of your patches). Which additionally give a useful message, such as: "This property does not apply when decorations are disabled" o I can see what you did there with the property bindings in the .ui file. I would prefer if this was done using the standard mechanics which also provide the explanitory tooltip when the properties are insensitive. But more importantly, Glade does not support the property GBinding GtkBuilder semantics, which means that we cannot really use Glade to edit the GladeHeaderBarEditor, that is more of a problem than the former issue. Tip: In case you were unable to edit Glade's editors with Glade, you need to './configre --enable-gladeui' You can then use the internal assets such as GladeEditorSkeleton, GladePropertyLabel, GladePropertyShell etc. Also, if you need interaction with some of the internal editors which are part of the GTK+ catalog (i.e. derive or embed other editors), then you can also add the gtk-private catalog to glade's palette which is found in plugins/gtk-private/glade-gtk-private.xml o The show-window-controls property should be set to 'ignore' Enabling the show-window-controls property in the workspace causes the automagick 'close' button to appear. Clicking on the automagick 'close' button on a headerbar in the workspace causes Glade's application close handling to occur (either Glade unexpectedly closes, or a popup comes up asking you if you are sure you want to close without saving). In conclusion, the glade-gtk-headerbar.c code needs attention more than anything else, especially wrt how placeholders are created, how the adaptor add_child()/remove_child()/replace_child() methods are implemented, and how the virtual "start-size" and "end-size" properties are broken. I would be willing to merge this without the "start-size"/"end-size" *IF* it can be done without crashes/errors, one drawback is that it would be impossible to acheive copy/cut/paste while preserving the number of placeholders in the copied headerbars (a copied headerbar would either have project widgets or a default of at least 1 placeholder on each side). However this is not how we've ever done things, so I can't really guarantee that following the road without "start-size" / "end-size" would not end in tears, so to speak. The correct/tried way is to drive properties with GladeCommands, and to drive placeholders in the adaptor when start-size/end-size is changed (in GladeWidgetAdaptor->set_property()).
Not sure I got it all 100% right, but I've pushed a number of fixes, and undo/redo feels a lot more stable now.
Today I worked on your headerbar branch, I needed to rebase and push a new branch because of a dependant core fix I pushed to master. First I fixed property sensitivity issues as you were calling glade_widget_property_set_sensitive() from the editor loading code and not from the adaptor code. See the following commit for details: commit c4fccac0405d88c7d3296c5047ad75401792f59b Author: Tristan Van Berkom <tristan@upstairslabs.com> Date: Wed Nov 5 18:38:55 2014 +0900 GtkHeaderBar: Fixed property sensitivity issues Then I fixed the core drag & drop logic so that packing properties are properly recorded from the source parent when dragging away, annoyingly it seems that drag & drop was not properly tested before merging that. The fix is in the following commit: commit 3f5e029817028334b349d80b7286d79426dff43c Author: Tristan Van Berkom <tristan@upstairslabs.com> Date: Wed Nov 5 19:55:07 2014 +0900 GladeCommand: Experimental - properly record child type for removal commands. Then I went on to try to fix your adaptor implementation and made a lot of progress, but it's still not quite done (note that the crashes I mentioned in my last review, the really critical part, were not yet fixed), the improvements are on your headerbar branch in the commit: commit f0092f1a7e38072eb193f005fe93136e0def2ab8 Author: Tristan Van Berkom <tristan@upstairslabs.com> Date: Wed Nov 5 20:08:32 2014 +0900 GtkHeaderBar: Improving child add/remove/replace support There are still serious issues with the adaptor, and it mostly revolves around the adaptor not creating placeholders dynamically to fit the data model's expectations of the "start-size" and "end-size" properties, and also not positioning child widgets properly when the "position" packing property is set. Here is a simple procedure to follow to reproduce the issues I'm talking about: o Create a new headerbar in the workspace o Add a button at the end of the headerbar o Drag the button from the end position and drop it in the start position (NOTE: You will need to select the headerbar first, since Drag&Drop wont take effect on an already selected widget) o Undo -> Widget goes back to the end as expected o Redo -> Widget goes back to the beginning as expected o Undo -> Now the widget goes back to the end, but now there is a button and a placeholder at the end, and no placeholder at the start o Redo -> Now the button is still in the project, but it's not added to the headerbar (it's just missing/gone/orphaned), and we have a headerbar with one placeholder on each side. The way things work in Glade with regards to children and positioning is that all data about child positioning is stored in packing properties on the modal. When Copy/Paste/Drag/Drop/Undo/Redo occurs, whenever a child is added to a container: o First time around ->replace_child() is called, this continues to be called so long as the placeholder exists, however the placeholder is transient and later can be destroyed (it's not project data). o After the core uses ->replace_child() the first time, packing property values after the widget insertion are introspected and saved for later use o Every subsequent time we add children, Glade follows the following rule: - Use GladeWidgetAdaptor->add_child() to get the widget into the container - Set the previously saved packing properties so that the widget is in the correct position and whatnot. From the adaptor side, it's your choice if you want to clear all the placeholders and fill in the gaps with new placeholders at ->add_child() and ->remove_child() time, or if you want to preserve the existing placeholders. The GtkGrid implementation is extreme in this regard and just wipes all existing placeholders and adds a complete new set of placeholders only in positions where no actual project child exists. Since the headerbar's position property is read-only for whatever reason, this presents a difficulty in preserving child order, there are two paths which can be followed: a.) Fix GtkHeaderBar so that the "position" property is writable and properly reorders it's children b.) Whenever ->add_child() or ->remove_child() is called on your adaptor, and whenever ->child_set_property() is called on your adaptor: You could remove everything, destroying the placeholders and re-add widgets in the proper order, adding a placeholder for every 'gap' so that you satisfy the 'start-size' / 'end-size' and that actual project widgets are found at the "position" which is stored in their packing property values (those which are stored in Glade's datamodel). I realize that this was originally somebody else's patch, and it's annoying to have to actually understand how adaptors work - however as you can see the previous patch was rather naive and the implementation of something like this is complex (hence why we never got around to writing all that code to support headerbar until this point). So, all I can say is that I'm sorry that it really costs work to get this working, I wish I could spend additional days working on this myself but I probably not do this again this year. I hope the guidance and work that I've put in so far is enough to push this through to a mergable state soon, it's very close aside for these important missing pieces.
Additional note: In glade-gtk-headerbar.c I've added a very simple debugging path, i.e. a commented out #define d(x) x, you can uncomment that to enable the tracing in that file if you find that usefull. Additionally, you can run Glade with: GLADE_DEBUG=commands:properties glade To get more tracing about what is happening when widgets are added and removed, and what (packing) properties are set to what values at undo/redo time. Hope this is useful.
Steps to reproduce the orphaned widget and critical message in the console: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - Create a headerbar in the workspace by dragging it from the palette into the workspace - Set the "size" property to 2 - Set the Custom Title property to TRUE - Add a GtkButton to the second placeholder Now the headerbar layout is: Placeholder | Button | Placeholder in the custom title - Set the Pack Type packing property of the button to End Now the headerbar layout is: Placeholder | Placeholder in the custom title | Button - Select the headerbar (so that the button is not selected) - Drag the button from the right side into the center Now the headerbar layout is: Placeholder | Placeholder | Button in the custom title - Undo until you reach the initial state of the added headerbar where there is only one placeholder - Redo one (this adds a placeholder) Now the headerbar layout is: Placeholder | Placeholder - Redo again (this sets custom title to TRUE) Now the headerbar layout is: Placeholder | Placeholder | Placeholder in the custom title - Redo again (this adds the GtkButton to the project) Now the button is in the project (see it in the inspector) but it does not show up in the workspace, additionally the critical message is printed in the console: Gtk-CRITICAL **: gtk_container_remove: assertion 'gtk_widget_get_parent (widget) == GTK_WIDGET (container) || GTK_IS_ASSISTANT (container) || GTK_IS_ACTION_BAR (container)' failed
I have entered the bug 739764 to track the issues with spurious ref counting imbalances related to drag & drop in placeholders. Noting that the above steps to reproduce only occur when dragging the button to add it to the headerbar, I'm sufficiently satisfied that this is in fact not a bug related to the headerbar adaptor in any way. I've went ahead and merged the headerbar branch now, all of the critical issues have been addressed and anything further would be an enhancement to the currently functional headerbar. Thanks Matthias for all your hard work on this.