Bug 147565 - Browsing comments in the navigation pane also browses hidden solved comments
Summary: Browsing comments in the navigation pane also browses hidden solved comments
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
7.1.8.1 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0 target:7.3.2
Keywords:
Depends on:
Blocks:
 
Reported: 2022-02-21 07:48 UTC by Anduril
Modified: 2022-06-10 03:17 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments
Navigation Pane comments arrows (28.76 KB, image/png)
2022-02-21 07:48 UTC, Anduril
Details
Example file (9.59 KB, application/vnd.oasis.opendocument.text)
2022-02-21 13:19 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Anduril 2022-02-21 07:48:44 UTC
Created attachment 178423 [details]
Navigation Pane comments arrows

Actual behaviour:
With the resolved comments view option unchecked and the resolved comments invisible, using the navigation pane's arrows (see screenshot attached), browses through resolved (invisible) comments as well (in this case nothing is shown but the user is taken to the place where the resolved comment is).

Expected behaviour
With the resolved comments view option unchecked, the the navigation pane's arrows should browse only through unresolved comments.

Reproduce
Reproduce steps in "Actual behaviour"
Comment 1 Telesto 2022-02-21 13:19:28 UTC
Created attachment 178430 [details]
Example file

1. Open example file
2. Uncheck View -> Resolved comments
3. Sidebar -> Navigator -> Notice 2 entry's (expected only the visible comments)
Comment 2 Telesto 2022-02-21 13:20:19 UTC
Repro
Version: 7.4.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 2bb10a827ac13d0caf009e8526ccd9f17dc71653
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: nl-NL (nl_NL); UI: en-US
Calc: CL Jumbo
Comment 3 Jim Raykowski 2022-02-24 03:31:08 UTC
Hi Anduril, Bug 146191 proposes to change the Navigate by Comment behavior to move the cursor to the comment anchor in the document versus moving it into the comment edit window. This would prevent the cursor from being lost when comment navigation is to resolved comments that are not shown. Not exactly what you have requested as expected behavior but probably better than the cursor disappearing.

The the move handler for comment navigation is here:
sw/source/uibase/uiview/viewmdi.cxx
IMPL_LINK( SwView, MoveNavigationHdl, void*, p, void )
case NID_POSTIT:
Comment 4 Jim Raykowski 2022-02-24 04:25:44 UTC
(In reply to Telesto from comment #1)
> Created attachment 178430 [details]
> Example file
> 
> 1. Open example file
> 2. Uncheck View -> Resolved comments
> 3. Sidebar -> Navigator -> Notice 2 entry's (expected only the visible
> comments)

@Telesto, separate ticket please so https://gerrit.libreoffice.org/c/core/+/130471 can be assigned to it.
Comment 5 Anduril 2022-02-24 10:06:03 UTC
Hi Jim,

thank you for coming back to me on this.

I have doubts on moving the cursor to the comment anchor rather than to the comment edit window, because when working, it is more useful to have the cursor in the edit window of the comment, which is where your attention (and action) is needed, rather than the spot in the text where the comment is anchored to (this is where you focus next, once you have read the comment).

The problem with the present behaviour is not really the disappearance of the cursor, since after a while you guess that it is going through all the comments (visible or invisible), but the fact that you are not able to focus on the non resolved.

Ex. Imagine a text where 90% of the comments have been resolved. If the collaborator only wants to see the non solved ones, he can either scroll the document visually, or use the navigation. In this latter case, he would like a similar experience to the one of scrolling, i.e. being brought from non solved to non solved comment.
Comment 6 Jim Raykowski 2022-02-26 00:39:40 UTC
Here is a patch that skips hidden comments when using the Navigate By control for Comments. It also fixes a recent issue that navigated comments are not scrolled into view:

https://gerrit.libreoffice.org/c/core/+/130518
Comment 7 Anduril 2022-02-26 08:37:53 UTC
Great patch Jim! This is what I was looking for! I imagine that this will be merged in some future version of Libreoffice and no further action from me is needed.
Comment 8 Commit Notification 2022-02-27 04:09:06 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5df0289f6af5aa7142017b56a8e17c134d54fe41

tdf#147565 Make navigate by comments skip hidden comments

It will be available in 7.4.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 Jim Raykowski 2022-02-27 04:12:51 UTC
(In reply to Anduril from comment #7)
> Great patch Jim! This is what I was looking for! I imagine that this will be
> merged in some future version of Libreoffice and no further action from me
> is needed.

You can test the patch by following instructions given here:
https://wiki.documentfoundation.org/Testing_Daily_Builds
Comment 10 Commit Notification 2022-03-06 18:46:51 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

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

tdf#147565 Make navigate by comments skip hidden comments

It will be available in 7.3.2.

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 11 bevis 2022-06-10 03:17:57 UTC
Thanks for your post, it's very helpful I hope in the future you will provide more information. I will visit and support the article for you https://drift-hunters.com