Similar to previous removals of g_cclosure_marshal_VOID__VOID we can remove
other marshallers for which are a simple G_TYPE_NONE with single parameter.
In those cases, GLib will setup both a c_marshaller and va_marshaller for
us. Before this commit, we would not get a va_marshaller because the
c_marshaller is set.
Related to GNOME/Initiatives#10
If we set c_marshaller manually, then g_signal_newv() will not setup a
va_marshaller for us. However, if we provide c_marshaller as NULL, it will
setup both the c_marshaller (to g_cclosure_marshal_VOID__VOID) and
va_marshaller (to g_cclosure_marshal_VOID__VOIDv) for us.
…from priv->button. My refactor to g_signal_disconnect_by_data()
included this widget, when I shouldn’t have as both modes use it.
This e.g. broke opening a CB by keyboard that was currently in menu
mode, if it had been in list mode initially (e.g. due to the theme).
Fix by moving to disconnect_by_func() and only removing in each mode’s
destroy() method the signals that it set on the button in its setup().
https://bugzilla.gnome.org/show_bug.cgi?id=788577
This reverts commit 1301723905.
This only appeared to fix the two bugs it linked because, rather than
being superfluous, the GTK+ grabs resulted in effectively having *none*,
or something, and could cause a critical when closing during a scroll.
This also reverts commit b9989e554b, which
depended on the above.
See next commit, which *should* properly fix what this one claimed to…
https://bugzilla.gnome.org/show_bug.cgi?id=787274
On clicking release, we call TreeView.get_path_at_pos() &, if we hit a
row, select it (if sensitive) & close the popup. But this alone does
not account for clicks on the expanders within the TreeView, so in
addition to expanding/collapsing, clicking them would close the list.
Check if the click is in the cell_area() & thus “excluding surrounding
borders and the tree expander area” but still including the background
(which TreeView.is_blank_at_pos() doesn’t); if TRUE, don’t select/close.
The popup doesn’t always resize enough… so there’s still breakage here.
The XXX comment on TreeView requests in list_position() may be relevant
to this. But at least this drags such CBs one step closer to adequacy:
expanding by mouse now works ~no worse~ than by keyboard already did.
https://bugzilla.gnome.org/show_bug.cgi?id=788505
ComboBox and TreeMenu warned in the doc for :row-span-column that the
value must not exceed :wrap-width, but :wrap-width does not interact
with the number of rows; it’s the :column-span-column that’s relevant.
Also: Warn that spans must be > 0 for rows too, and that column spans <=
:wrap-width are also not useful for items at menu column positions > 0.
Finally, refer to items having spans, not values, as we were already
talking about values in the model (and rows in the menu).
On button release, we were popping down if the event widget was anything
but priv->button. This broke scrolling by clicking a mouse button, i.e.
when releasing a click in the trough or finishing a drag of either bar.
That’s unexpected, inconvenient, and pointless. So, let’s stop doing it.
https://bugzilla.gnome.org/show_bug.cgi?id=738893
Various disconnections had the wrong flags and/or data, so we failed to
disconnect a pile of signals, shown by 0 returned by the disconnect_*()
functions. Fix this, and use the nicer disconnect_by_*() while here.
set_transient_for(toplevel) was only called in list_setup(). It was easy
to make a test showing a NULL :transient-for instead of the correct one.
So, move the call from list_setup() to popup_for_device(). Also do that
for window_group_add_window(), which means not calling it redundantly.
(I tried using a ComboBox:parent-set handler, but the Inspector’s CB
didn’t like that: it calls popup_for_device() twice and closes on button
release. Anyway, using popup() is much more concise than a new handler.)
The screen for the list-mode popup_window was only being set in
set_popup_widget(), i.e. when changing modes, so if the ComboBox was
moved to a different screen later, the popup would appear on the
original one, which is wrong.
Worse, this (somehow) broke opening some combos in the Inspector.
Fix this by moving the call to set_screen() to popup_for_device(), so
the popup_window is put on the correct screen each time around.
https://bugzilla.gnome.org/show_bug.cgi?id=468868https://bugzilla.gnome.org/show_bug.cgi?id=786771
Bad actors, such as our very own FileChooserButton, may connect to the
:popped-up property and alter the model as the menu becomes in/visible.
We were getting an iter to the model while popped-up, then doing
popdown(), then using the iter, which may have just been invalidated by
the errant notify::popped-up handler. If so, we quickly crash fatally.
This is clearly bonkers, but until such patterns are removed, we have to
work around them. So, set_active() from the clicked item while it is
known to be valid, by moving the call to set_active() before popdown().
While here, change set_active_iter(iter) to set_active_internal(path) to
avoid pointlessly going through the iter to get the path we already have
https://bugzilla.gnome.org/show_bug.cgi?id=729651
combo_box_popdown() of course doesn’t popdown our menu if it is NULL.
But the required call to this at end-of-life was in destroy(), by which
point dispose() already NULLed the menu, so Menu::popdown() would never
run, even if it should. Fix this by trying popdown() earlier in unmap().
Also, add a converse assurance that we don’t popup() while not mapped.
i.e. when wrap-width > 0. This was only being done for non-grid cases.
So, ComboBoxes in grid mode did not indicate their selection when popped
up and required users to keynav from ‘nothing’ (at the top-left) to the
item they wanted to select. By selecting the active item in advance, now
it’s highlighted & acts as the starting point for keynav around the grid
GtkFileChooserButton installs a handler for the popped-up signal, which
refilters the menu, in order to hide the “(None)” item from the popup
if it was previously selected in the ComboBox. This oddity means that:
• Until recently, this item would be selected in the menu shell, which
would then be popped up and change the selection away from that item.
This was therefore redundant (more on which below!) but benign.
• After the patch for https://bugzilla.gnome.org/show_bug.cgi?id=771242
however, this causes a critical assertion fail, as now we stash the
originally selected item in a pointer so that it can be selected only
after realisation/popup – but by that stage, the model has just been
refiltered and the previous pointer no longer refers to a valid item.
This commit works around this problem by, after popping up the menu,
getting the active item again, in case a popped-up handler has gone and
invalidated the pointer to the active item that we saved before popup.
If a handler does this, everything done to find/use the original item is
pointless. But this avoids the ugly critical in FileChooserButton, while
not harming every other ComboBox that doesn’t mess with its model while
popping up (hopefully the vast majority), and it’s very difficult to
imagine a way to check if the active item is /going to/ be hidden later)
For a menu mode CB with wrap_width == 0 and an active item, that item is
selected in gtk_combo_box_menu_popup. Selection causes the MenuShell to
activate and hence take a grab. This was done before the menu was popped
up. A patch distributed in Debian sid - after being proposed on our BZ -
revealed that on the 1st popup of any such ComboBox, within grab_add,
the MenuShell's toplevel's GdkWindow is NULL. This causes a Gdk-CRITICAL
assertion fail on the 1st time opening any such CB, on Debian and if
that patch were merged to GTK+. By selecting after popup, we ensure the
MenuShell is realised before its grab_add and so avoid the critical.
https://bugzilla.gnome.org/show_bug.cgi?id=771242
