GNOME Bugzilla – Bug 124016
On a autohide, parent panel goes down with its open drawers
Last modified: 2004-12-22 21:47:04 UTC
I am creating this bug to further re-scope bug 121382.
Created attachment 20531 [details] [review] First attempt
Mark: This works quite fairly. At this point I am not sure if all the combinations work. Well, that's precisly the reason the HEAD is for. ;-)
Patch looks pretty good. Some comments: + I think you'll need to propogate the autohide disabler up to parent drawers ... i.e. something like: static void panel_toplevel_push_autohide_disable (PanelToplevel *toplevel) { g_return_if_fail (toplevel != NULL); toplevel->priv->attach_toplevel_count++; if (toplevel->priv->attach_toplevel) panel_toplevel_push_autohide_disable ( toplevel->priv->attach_toplevel); } + The function names should be push_autohide_disabler, pop_autohide_disabler. + Rename attach_toplevel_count to n_autohide_disablers. + Rename panel_toplevel_attach_toplevel_count to panel_toplevel_get_autohide_disabled and make it return a bool. + No need to use panel_toplevel_get_attach_toplevel in the code. Can just use toplevel->priv->attach_toplevel. + In pop_autohide_disabler you'll want a pre-condition that n_autohide_disablers > 0
Created attachment 20534 [details] [review] With comments incorporated.
> + I think you'll need to propogate the autohide disabler up to > parent drawers ... Count on the base drawer attached to the panel should be enough. Drawers holding other drawers work well without this. Is there something I am missing ?
Ah yes, I see what you're saying - good point. A couple of tiny comments - feel free to commit after you fix these up: + g_return_if_fail (toplevel != NULL); - you've two spaces between the parentheses + if (toplevel->priv->n_autohide_disablers > 0) - this should be an assertion - i.e. g_return_if_fail (toplevel->priv->n_autohide_disablers > 0);
Fixed in HEAD and gnome-2-4 branches. Thanks.
Is there a "final" patch that incorporates fixes to the last comments? Could it be posted here? TIA.
I have cleaned up stuff on my system. But it should not be great work to provide you one. But just wonderning why you needed it ? It's already in the CVS.
To get it into Fedora Core 1 (patching against current version). I guess I could check the latest patch here against current code and see what final changes you made, but if you can provide diff between the two relevant CVS versions that of course would be easier (for me ;).
I don't understant: is it not ok to use the latest 2.4.x gnome-panel in Core 1? This is a stable release so it contain only bugfixes (which you might need to do yourself too).
As I am not in charge of FC1 development I want to propose an update of gnome-panel, but I want to be able to offer different upgrade paths: Either use available patches or upgrade to 2.4.2. I am not sure which would be easier to achieve on FC1 (not sure how much FC1 specific patches would trouble upgrading to 2.4.2). Having the complete patch to test before proposing anything would be nice, but if that is too much of a hassle I am probably able to produce it using the patch attached here and the latest CVS.
Can I generate a diff -u using Bonsai? If so how? Which versions should I diff to retrieve this patch? 1.37 and 1.38? Any other files affected in the final version that are not in the attached diff (ie any file other than gnome-panel/ChangeLog and gnome-panel/panel-toplevel.c)?
Sorry for the late answer. You can see a diff there: http://cvs.gnome.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvs/gnome&subdir=gnome-panel/gnome-panel&command=DIFF_FRAMESET&file=panel-toplevel.c&rev2=1.38&rev1=1.37 However, there were more bugs fixed and I really recommend that you package latest 2.4.x package.