Bug 152517 - Navigator: Ctrl+Minus collapses the whole Headings tree, not the selected node
Summary: Navigator: Ctrl+Minus collapses the whole Headings tree, not the selected node
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.6.0 target:7.5.0.0.beta2
Keywords:
Depends on:
Blocks: Navigator
  Show dependency treegraph
 
Reported: 2022-12-15 09:51 UTC by Mike Kaganski
Modified: 2023-04-21 02:50 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2022-12-15 09:51:21 UTC
Create a text document with several headings, subheadings, and sub-subheadings (using Heading 1, Heading 2, and Heading 3 paragraph styles), like

1
1.1
1.1.1
1.2
1.2.1
2
2.1
3
3.1.1
3.1.2
3.2

In Navigator, there will be shown a respective Headings tree, where you can expand and collapse some nodes.

Now check that pressing simple [+] (Plus) keyboard key expands the immediate children of the current node in the Navigator (keeping the subnodes state unchanged); [Ctrl]+[+] expands all the subnodes of the current node, irrespective of their prior state (this is the *correct* and expected behavior).

E.g., if you collapse all sub-(sub-)nodes from bottom to top (so that every node was collapsed initially), and then put cursor on node "1" and press [+], the node "1" will expand to show "1.1" and "1.2", but not "1.1.1" nor "1.2.1"; and also nodes "2" and "3" will be still collapsed. If you collapse node "1", and press [Ctrl]+[+], the whole tree under node "1" will expand, showing you "1.1.1" and "1.2.1", but nodes "2" and "3" will be still collapsed.

A note: you may see that the state of subnodes is kept when you press [+], if you first expand the whole node (select "1" and press [Ctrl]+[+]), then collapse node "1.1" and node "1" (do not collapse node "1.2"), then select "1" and press [+]: this will expand the "1", showing "1.1", "1.2", and "1.2.1" (as it was still expanded when "1" got collapsed).

Pressing simple [-] (Minus) keyboard key collapses the immediate children of the current node (keeping the subnodes state unchanged), and this is also *correct* and expected. To summarize, [+] and [-] keyboard keys work exactly as clicking on the respective "+" and "-" icons to the left of the selected node acts.

Now the problem: Pressing [Ctrl]+[-] keyboard keys collapses the *whole Headings top-level element in the Navigator*, no matter where in the subtree you are. You may have selected "2.1", or "1", and after the [Ctrl]+[-], only the top-level "Headings" element is visible, all the sub-elements are collapsed. This is unexpected and inconsistent - both compared to how the [Ctrl]+[+] counterpart works, and also to the behavior of other expandable lists - see e.g. how Styles sidebar panel works, when Hierarchical view is selected (pressing [Ctrl]+[-] on Text Body node collapses its children, but not top-level nodes, nor siblings like Index).

Tested using Version: 7.4.3.2 (x64) / LibreOffice Community
Build ID: 1048a8393ae2eeec98dff31b5c133c5f1d08b890
CPU threads: 12; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: ru-RU (ru_RU); UI: en-US
Calc: CL

I don't know if these key combinations work the same on other OSes.
Comment 1 Jim Raykowski 2022-12-16 05:51:01 UTC
[Ctrl]+[+] and [Ctrl]+[-] aren't handled by SwContentTree directly but are passed through to welded TreeView backends. Gtk3 doesn't do anything for these. For gen type backends, [Ctrl]+[-] is handled here[1].

[1] https://opengrok.libreoffice.org/xref/core/vcl/source/treelist/svimpbox.cxx?r=5f9cd841#2393
Comment 2 Jim Raykowski 2022-12-16 06:04:05 UTC
I should have checked the history there. It's seems I made this behavior. So, what is the recommended behavior for [Ctrl]+[-]? Perhaps collapse all sub nodes along with the current node?
Comment 3 Mike Kaganski 2022-12-16 06:26:54 UTC
(In reply to Jim Raykowski from comment #2)
> So, what is the recommended behavior for [Ctrl]+[-]? Perhaps collapse all
> sub nodes along with the current node?

The currently selected node must stay selected, but all its children must be collapsed *recursively*, so that the selected node must get the [+] icon at the left, and all its (now hidden) children also must have [+] if they have subchildren - so when you later press [+] at the parent, the previously expanded children show collapsed.
Comment 4 Jim Raykowski 2022-12-16 06:40:15 UTC
(In reply to Mike Kaganski from comment #3)
> The currently selected node must stay selected, but all its children must be
> collapsed *recursively*, so that the selected node must get the [+] icon at
> the left, and all its (now hidden) children also must have [+] if they have
> subchildren - so when you later press [+] at the parent, the previously
> expanded children show collapsed.
I think effort here does this:
https://gerrit.libreoffice.org/c/core/+/144286
Comment 5 Mike Kaganski 2022-12-16 07:00:06 UTC
(In reply to Jim Raykowski from comment #4)
> https://gerrit.libreoffice.org/c/core/+/144286

Thanks!

Interesting, why the styles list behaves as expected - does it duplicate the code?
Comment 6 Jim Raykowski 2022-12-16 07:46:08 UTC
(In reply to Mike Kaganski from comment #5)
> Interesting, why the styles list behaves as expected - does it duplicate the
> code?
Yes, very interesting. It seems the styles list uses nullptr as the parent for all entries[1] which makes[2] work as expected because of[3].

[1] https://opengrok.libreoffice.org/xref/core/sfx2/source/dialog/StyleList.cxx?r=ef862ba4#949
[2] https://opengrok.libreoffice.org/xref/core/vcl/source/treelist/svimpbox.cxx?r=5f9cd841#2393
[3] https://opengrok.libreoffice.org/xref/core/vcl/source/treelist/treelist.cxx?r=6e2bd112&mo=29042&fi=1020#1020
Comment 7 Jim Raykowski 2022-12-16 20:51:20 UTC
(In reply to Jim Raykowski from comment #6)
> Yes, very interesting. It seems the styles list uses nullptr as the parent
> for all entries[1] which makes[2] work as expected because of[3].
> 
> [1]
> https://opengrok.libreoffice.org/xref/core/sfx2/source/dialog/StyleList.
> cxx?r=ef862ba4#949
> [2]
> https://opengrok.libreoffice.org/xref/core/vcl/source/treelist/svimpbox.
> cxx?r=5f9cd841#2393
> [3]
> https://opengrok.libreoffice.org/xref/core/vcl/source/treelist/treelist.
> cxx?r=6e2bd112&mo=29042&fi=1020#1020

After further investigation, it seems there is something not right with SvTreeList::GetRootLevelParent. The only place I could find it called from is[2]. TreeListBox::GetRootLevelParent seems correct though[4].

[4] https://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/control/dbtreelistbox.cxx?r=5f6596bd#485
Comment 8 Commit Notification 2022-12-17 05:08:18 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/eef94d7b4360e17ba21577fe52bd60214bd5280c

tdf#152517 Fix gen backend treeview ctrl+minus behavior

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2022-12-20 06:10:32 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

https://git.libreoffice.org/core/commit/ffd8d368b3a780c7b687512540821ad52f2db457

tdf#152517 Fix gen backend treeview ctrl+minus behavior

It will be available in 7.5.0.0.beta2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Jim Raykowski 2023-04-21 02:50:20 UTC
Bit late in setting this one as fixed. It appears to be working as expected in:

Version: 7.6.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 80a0d3b44cd6edb6419ab66588548ba169f2567b
CPU threads: 4; OS: Linux 5.15; UI render: default; VCL: x11
Locale: en-US (C); UI: en-US
Calc: threaded