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 788505 - ComboBox in appears-as-list mode: Clicking a TreeView expander also selects that row/closes the popup
ComboBox in appears-as-list mode: Clicking a TreeView expander also selects t...
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkComboBox
3.22.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2017-10-04 10:15 UTC by Daniel Boles
Modified: 2017-10-05 00:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ComboBox: list: Fix expanding/collapsing tree rows (2.70 KB, patch)
2017-10-04 21:37 UTC, Daniel Boles
committed Details | Review
ComboBox: list: Don’t leak path on expand/collapse (2.04 KB, patch)
2017-10-05 00:33 UTC, Daniel Boles
committed Details | Review

Description Daniel Boles 2017-10-04 10:15:45 UTC
LISTMODE=1 ./tests/testcombo

Open the "What are you ?" ComboBox, hover over one of the expanders, see that it prelights, and click on it, expecting just to reveal the items within that node.

The ComboBox closes, and the parent row whose expander you clicked is selected (if sensitive).

Of course, clicking the expander should only cause its node to be expanded/collapsed, nothing else. Otherwise, tree models in list mode are next to useless.
Comment 1 Daniel Boles 2017-10-04 10:33:00 UTC
This probably occurs because gtk_combo_box_list_button_released() doesn't bother to check whether gtk_tree_view_is_blank_at_pos() for the clicked position, which should return TRUE if an expander or other non-body area is clicked in that row.
Comment 2 Daniel Boles 2017-10-04 18:06:29 UTC
but! Checking if blank_at_pos() means we also lose the ability to click on any non-text (etc.) parts of the row, which is a pain for usability.

Thankfully, though, get_cell_area() "returns only the cell itself, excluding surrounding borders and the tree expander area." So, we can take the output x coordinate from the event and bail out if it's part of that excluded area. Clicks on the blank ("background") part of the row still work and select the row, but clicks on the expander are ignored as intended.

...but. This then reveals another problem: the popup doesn't resize to accomodate the newly revealed rows, so they can end up being unusable. I guess that'll require a reallocate or such.

-appears-as-list: the ride never ends.
Comment 3 Daniel Boles 2017-10-04 20:05:17 UTC
There's gtk_combo_box_list_popup_resize(), which seems like what's needed, but it doesn't seem to be doing the trick in this case.

The gremlins. They're everywhere.

This is most noticeable e.g. when expanding the middle row of the testcombo CB already mentioned above, but it can affect rows in other locations too.

I'll probably push what I have, as it's an *improvement*. It makes clicking work, to the limited extent that expanding with the keyboard already did.
Comment 4 Daniel Boles 2017-10-04 21:37:26 UTC
Created attachment 360931 [details] [review]
ComboBox: list: Fix expanding/collapsing tree rows

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.
Comment 5 Daniel Boles 2017-10-04 22:02:39 UTC
Attachment 360931 [details] pushed as a20ff44 - ComboBox: list: Fix expanding/collapsing tree rows
Comment 6 Daniel Boles 2017-10-05 00:33:50 UTC
Created attachment 360941 [details] [review]
ComboBox: list: Don’t leak path on expand/collapse

Determining that we clicked on the expander means that we had a valid
path, so we still need to free that, even though we’re not selecting it.
Comment 7 Daniel Boles 2017-10-05 00:36:34 UTC
Comment on attachment 360941 [details] [review]
ComboBox: list: Don’t leak path on expand/collapse

Attachment 360941 [details] pushed as 34cd1e3 - ComboBox: list: Don’t leak path on expand/collapse