Bug 120290 - Clicking behind a sentence ending in a commented word places cursor before final period
Summary: Clicking behind a sentence ending in a commented word places cursor before fi...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: All All
: medium minor
Assignee: Not Assigned
URL:
Whiteboard: target:7.1.0 target:7.2.0
Keywords: bibisected, regression
: 132192 136872 (view as bug list)
Depends on:
Blocks: Text-Cursor
  Show dependency treegraph
 
Reported: 2018-10-03 17:15 UTC by Michael Büker
Modified: 2021-01-18 07:04 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
comment.odt: cursor not placed at end of the line - doesn't depend on selection, but just on the anchor position. (11.28 KB, application/vnd.oasis.opendocument.text)
2019-02-01 11:30 UTC, Justin L
Details
comment.odt: tweaked to test the thinest character (|), and multi-comments. (13.33 KB, application/vnd.oasis.opendocument.text)
2020-11-12 13:32 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Büker 2018-10-03 17:15:01 UTC
Description:
When a line ends with a period, and the final word before that period is commented, clicking to the right behind the end of the line (in the white space behind the final period) places the cursor *before* that period, not behind.

Steps to Reproduce:
1. Open an empty Writer document.
2. Enter a sentence ending in a period, like: "This is a sentence."
3. Add a comment to the word "sentence".
4. Click in the white space to the right of the sentence.

Actual Results:
The cursor is placed between the word "sentence" and the period.

Expected Results:
The cursor is placed at the end of the line, behind the final period.


Reproducible: Always


User Profile Reset: Yes



Additional Info:
Comment 1 Dieter 2018-10-03 19:12:25 UTC
I confirm this with

Version: 6.2.0.0.alpha0+ (x64)
Build ID: 48cfa0b00b22f11ade53aec79b2fdddad253e1bd
CPU threads: 4; OS: Windows 10.0; UI render: GL; VCL: win; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2018-10-03_02:01:42
Locale: en-US (de_DE); Calc: CL
Comment 2 Xisco Faulí 2018-10-04 15:11:22 UTC
Reproduced back to

Version 4.1.0.0.alpha0+ (Build ID: efca6f15609322f62a35619619a6d5fe5c9bd5a)

but not in

LibreOffice 3.3.0 
OOO330m19 (Build:6)
tag libreoffice-3.3.0.4
Comment 3 Aron Budea 2018-11-12 05:17:05 UTC
(In reply to Xisco Faulí from comment #2)
> but not in
> 
> LibreOffice 3.3.0 
In 3.6.0.4 the comment is anchored to the starting point, not to the full word, this changed with 4.0.0.3, and the bug was also introduced in that version.
Comment 4 shani 2018-11-29 09:40:01 UTC
Still exists in version 6.3.0.0.alpha0+ (x64)

Build ID: 0f25a3c36f27fd51453b9a9115f236b83c143684
CPU threads: 8; OS: Windows 10.0; UI render: GL; VCL: win; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2018-11-27_20:06:55
Locale: zh-TW (zh_TW); UI-Language: en-US
Calc: threaded
Comment 5 Justin L 2019-01-31 07:08:52 UTC
cleanly bibisects to July 20-22, 2012 between bad commit 51065497ea83e90764860784dc6e193faaf0d673
and good commit 06f20d73da21342046a480a6b22af69901351328

but I didn't immediately see a likely culprit.
Comment 6 Justin L 2019-01-31 18:18:36 UTC
commit 2fe8ce572222e90327cfeed6b7da07a2c57bdbbb
Author: Miklos Vajna
Date:   Fri Jul 20 14:41:23 2012 +0200
    SwXTextField::attachToRange: don't generate new fieldmark id on each import

NOTE: comment 3's note about the word being selected/highlighted by the comment is the same commit.
Comment 7 Justin L 2019-02-01 11:30:42 UTC
Created attachment 148844 [details]
comment.odt: cursor not placed at end of the line - doesn't depend on selection, but just on the anchor position.

When determining the width of the line, the comment anchor is considered to be two spaces wide.
sal_uInt16 SwViewOption::GetPostItsWidth( const OutputDevice *pOut )
{    return sal_uInt16(pOut->GetTextWidth("  "));  }

When the anchor is near the end of the line, this fake space can be larger than the fullstop width, and so it thinks that it has found the correct location for the cursor.

This hack fixes the bug... itrcrsr.cxx SwTextCursor::GetCursorOfst()
        if ( pPor->IsPostItsPortion() )
+       {
            nWidth30 = 30 +  pPor->GetViewWidth( GetInfo() ) / 2;
+           if ( nX && nWidth30 > nX )
+               nWidth30 = nX-1;
+       }

This "if it is a comment portion" section was added by https://cgit.freedesktop.org/libreoffice/core/commit/?id=dca044e9572144c1ba9ce423b10f7a5456837cda
Comment 8 Justin L 2020-11-12 13:32:41 UTC
Created attachment 167239 [details]
comment.odt: tweaked to test the thinest character (|), and multi-comments.

Miklos cautiously handled a similar issue in https://cgit.freedesktop.org/libreoffice/core/commit/?id=27c46ecf34b32bae1806d3fc0e1684179301feb8 for comments after flies. I guess in theory it could be extended for any portion with width zero. 

Handling a comment (or other zero width portions) at the very end of the line can be accomplished with:
    if (nWidth30 >= nX && (!pPor->IsFlyCntPortion() || !pPor->GetNextPortion()->IsPostItsPortion()))
-        return false;
+        return nWidth30 == nX && !pPor->GetNextPortion()->Width();
 
But who knows what all portions can have zero width, and that could have a LOT of unintended consequences. Also, this routine is called every time the cursor quivers - so tight coding is very important.

Proposed fix:
https://gerrit.libreoffice.org/c/core/+/105746 tdf120290 sw: move cursor behind comment at line end
https://gerrit.libreoffice.org/c/core/+/105747 tdf#120290 sw: comment no longer needs any width
Comment 9 Commit Notification 2020-11-18 07:04:37 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/63d8041c60c5d7c63dd34f94a694871b7ea9e2ca

tdf120290 sw: move cursor behind comment at line end

It will be available in 7.1.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 10 Commit Notification 2020-11-23 10:14:00 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3c4b09a250605c9d73e48df83a6ee36a8bc2fb8b

tdf#120290 sw: comment no longer needs any width

It will be available in 7.2.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 11 Justin L 2020-11-23 10:37:12 UTC
I'm happy to not have this fixed until 7.2. That way if the first patch - which is the more dangerous - causes problems in 7.1, it is easier to revert the whole thing.
Comment 12 Justin L 2020-12-16 06:16:03 UTC
*** Bug 136872 has been marked as a duplicate of this bug. ***
Comment 13 Telesto 2020-12-24 09:13:11 UTC
*** Bug 132192 has been marked as a duplicate of this bug. ***
Comment 14 Dieter 2021-01-18 07:04:56 UTC
VERIFIED FIXED with

Version: 7.2.0.0.alpha0+ (x64)
Build ID: 9f9798f07f0b56ae474f31ded671cc8da598d244
CPU threads: 4; OS: Windows 10.0 Build 19042; UI render: Skia/Raster; VCL: win
Locale: de-DE (de_DE); UI: en-GB
Calc: threaded

Justin, thanks for fixing that issue!