Steps to reproduce: 1. Open Writer 2. Insert an image or a drawing object 3. Open the navigator (F5) 4. Double click on images or drawing object Observed behaviour: ListTree is not expanded. Reproduced in Version: 6.1.0.0.alpha0+ Build ID: b36ea44dcbdb862b0ac6e6cdaf27225b43dc0c7e CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); Calc: group
Regression introduced by: author Jim Raykowski <raykowj@gmail.com> 2018-01-01 14:52:41 -0900 committer Thorsten Behrens <Thorsten.Behrens@CIB.de> 2018-01-11 11:30:08 +0100 commit 582b2ed5ba25657afc2c2d45860899325b3b2450 (patch) tree 60ec98ab10f80fb2da3a08ab8beb3d73e3c344b7 parent f66fbd947f70f6be6b22ab372facaeb9e2fb63ae (diff) tdf#36308 make double click not expand/collapse node in Navigator tree Bisected with: bibisect-linux64-6.1 Adding Cc: to Jim Raykowski
Hi Xisco, The intent of tdf#36308 was to do just this. It requires that the tree node icon to be clicked to expand/collapse. For Headings navigation this has been considered an improvement. I agree that this is a regression of behavior for the other navigation elements. Here is a patch that provides for double click to expand/contract for navigation elements other than content type Headings: https://gerrit.libreoffice.org/#/c/53074/ This has required a change to a recent patch commit 1b9af08481b8f7f4bd15a30508606dff56b8e74f Author: Jan Holesovsky <kendy@collabora.com>, Tue Mar 13 12:39:11 2018 +0100 (5 weeks ago) Committer: Jan Holesovsky <kendy@collabora.com>, Tue Mar 13 16:28:40 2018 +0100 (5 weeks ago) Follows: libreoffice-6-0-branch-point Branches: <Expand> tdf#116334: Actually when there is no handler, we have to return 'true'. Also use the return value of the link's Call() [as the SvTreeListBox::ExpandingHdl() is doing], the appropriate callbacks seem to return the expected 'false' in the cases I've reviewed... Change-Id: I0cdd63e8ec4c794839070b914150e0b32f743359 Reviewed-on: https://gerrit.libreoffice.org/51211 Tested-by: Jenkins <ci@libreoffice.org> Reviewed-by: Jan Holesovsky <kendy@collabora.com> to what the patched code was two patches prior treelistbox.cxx:428 bool SvTreeListBox::DoubleClickHdl() { aDoubleClickHdl.Call( this ); return true; } This is because if SvTreeListBox::DoubleClickHdl() does not return true svimplbox.cxx:2024 if( pView->DoubleClickHdl() ) will not be entered and svimplebox.cxx:2041 if( bSubLstOpDblClick ) can not be checked to decide if the element is one where it's sublist should be expanded/collapsed on doubleclick. We better check with Jan on this.
Hi Jim, Thanks for the detailed explanation. I reported it because it looked like a bug to me, but as you're saying, the logic behind 582b2ed5ba25657afc2c2d45860899325b3b2450 is to disable the expand/collapse, thus, this bug will be a NOTABUG. Whether we should enable it for all elements other than headings or not, I would prefer to hear the UX team's opinion first...
Not sure if we should implement different interactions (requested here is to keep the change for headings where the node state is retained on double-click but to open the parental nodes in all other cases, e.g. Images). The user should learn whether double-click expands or not. Any dependency on the type of content, even when it's perfectly clear in this case, adds complexity to the UI and not all beginners can follow the reasoning behind different workflows. IMHO, WF (open for discussion)
When I orignally read this report I was of the opinion WF becuase we fixed it already. After testing the patch I was more in favor of using it's behavior. As far as I know double click is used to expand/collapse all other lo tree lists but the one in the Navigator.
@Heiko, could it be discussed in tomorrow's UX meeting ?
testing in Version: 6.1.0.0.alpha0+ Build ID: 66c02d16dd078613e754dcc775f366413fad13f0 CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk2; TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-04-20_09:08:39 Locale: nl-NL (nl_NL.UTF-8); Calc: group I see no different behavior for Headings, so remove "( other than the headings )" from the summary
We discussed the topic in the design meeting and came to the conclusion to follow Jim's comment 5. Root nodes (including "Headings" but not the actual headings below) open on double click, child nodes don't.
Thanks, Jim!
Jim Raykowski committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=b649ce123dea372359ec571135a68eb3de844e5b tdf#117063 Modify tree list double click behavior in the navigator It will be available in 6.1.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Verified in Version: 6.1.0.0.alpha1+ Build ID: 43af818557904450b13839350c65ad865b9ee9d2 CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); Calc: group
Jim: Thank you for working on this - it's pity you didn't CC me in this bug when you've done the analysis in comment 2... Returning true unconditionally in SvTreeListBox::DoubleClickHdl() I'm afraid re-introduces bug 115950. The root of the problem here is that many of the double-click handlers return a wrong value. Now I have a list of places where that happens thanks to a compiler plugin, so I'll go through them, and fix accordingly - then I'm likely to change the code in SvTreeListBox::DoubleClickHdl() back to what it was before your b649ce123dea372359ec571135a68eb3de844e5b . Seeing the amount of regressions, I agree that for the moment it is better to return true unconditionally for the moment :-) - thanks for that!
(In reply to Jan Holesovsky from comment #12) > Jim: Thank you for working on this - it's pity you didn't CC me in this bug > when you've done the analysis in comment 2... Jan, Sorry about that. I thought one of the bugzilla gurus would get a message to you. They do an outstanding job. Many kudos to them! I'm still sort of a beginner here and don't know how to CC.
Jim - don't worry at all! You are doing great, and it is awesome to have you contributing code :-) - thank you for that!
Eike Rathke committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=c9d1655d455ad783694e6d7f0d2e6cf3d0d0acae Resolves: tdf#115950 proper double click return and bail out, tdf#117063 It will be available in 6.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Eike Rathke committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=08ac6e6f991cffcb1fed7bd1bb4ec67e94817507 SvTreeListBox::DoubleClickHdl: Navigator return more to be done, tdf#117063 It will be available in 6.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Eike Rathke committed a patch related to this issue. It has been pushed to "libreoffice-6-1": http://cgit.freedesktop.org/libreoffice/core/commit/?id=172817aa670ff923780d0159c4416871179fa94c&h=libreoffice-6-1 Resolves: tdf#115950 proper double click return and bail out, tdf#117063 It will be available in 6.1.0.2. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Eike Rathke committed a patch related to this issue. It has been pushed to "libreoffice-6-1": http://cgit.freedesktop.org/libreoffice/core/commit/?id=c73b9d69abf1b26dc2c6f9d6700b2a5f2c9e6f36&h=libreoffice-6-1 SvTreeListBox::DoubleClickHdl: Navigator return more to be done, tdf#117063 It will be available in 6.1.0.2. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.