Bug 57938 - First character before ranged comment cannot be edited or removed
Summary: First character before ranged comment cannot be edited or removed
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.0.0.0.alpha0+ Master
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Miklos Vajna
URL:
Whiteboard: target:4.1.0 target:4.0.0.2
Keywords: regression
Depends on:
Blocks:
 
Reported: 2012-12-06 07:25 UTC by K Udo Schuermann
Modified: 2013-08-22 06:15 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Demonstrates ranged comment issue (14.04 KB, application/vnd.oasis.opendocument.text)
2012-12-07 19:56 UTC, K Udo Schuermann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description K Udo Schuermann 2012-12-06 07:25:35 UTC
The character immediately preceding the start of a ranged (multi-character enclosing) comment cannot be deleted or replaced, and no character can be inserted immediately before a ranged comment. Example text:

X[abcd]other

Where "[abcd]" represents a ranged comment (highlighted in yellow). Attempts to delete "X" are silently ignored. Attempts to add text immediately before "[abcd]" produce a dialog with the complaint that "Readonly content cannot be changed. No modifications will be accepted".

Observed in 4.1.0.0.alpha0+ (Build ID: bf5cf1a30bad7d831a501e5bf037ed83df8b200)
Comment 1 Joel Madero 2012-12-06 17:04:43 UTC
can you please attach a document that displays this behavior? Also provide step by step instructions on how to reproduce. Going to mark as NEEDINFO, once you provide these two things please change back to UNCONFIRMED and we'll take a look at it
Comment 2 K Udo Schuermann 2012-12-07 19:56:52 UTC
Created attachment 71152 [details]
Demonstrates ranged comment issue

How to reproduce:

1. Open LibreOffice Writer (devbuild of 4.1, see also original bug report)
2. Type a sequence of characters (example: "Test", keep it to one line for simplicity)
3. Highlight all text (shortcut Ctrl-a)
4. From "Insert" menu select "Comment" (shortcut Ctrl-Alt-C)
5. Type at least one character into the margin comment
6. Click back into the main document's text
7. Press [Home] key to move cursor to very front of document (before comment)
8. Press a character (letter, number) of your choice.

Expected behavior: Chosen character is inserted (outside/before comment range)

Observed behavior: Dialog appears, stating "Readonly content cannot be changed. No modifications will be accepted".

NOTE: Dialog is labeled "LibreOffice 4.1" as this is a devbuild (see build ID in original report).

NOTE: It appears that any ranged (1+ length) comment exhibits this behavior, no matter where it is located (start of a line/paragraph, middle, whatever). The "old" style comments (single position/marker) are fine.
Comment 3 Joel Madero 2012-12-07 20:01:11 UTC
Confirmed, marking:


New (Confirmed)
Whiteboard (regression)
Normal (can prevent high quality/professional work)
Highest (regression + common feature (comments) + incorrect error message)


Thanks for reporting and keeping this up to date. 

Version 4.1.0.0.alpha0+ (Build ID: dfd474710f959dcf78038deb990c3618b467ec6)
Comment 4 Joel Madero 2012-12-07 20:01:59 UTC
@K Udo Scheurmann: you're handling this bug? It's assigned to you so I just want to be sure
Comment 5 K Udo Schuermann 2012-12-07 20:14:33 UTC
I will have a look (as would love to contribute!) but my experience with the code base is virtually nil, which is to say I only ever submitted a typo fix that I happened across while looking for something else that I never found.
Comment 6 Joel Madero 2012-12-07 20:15:45 UTC
I'm going to go ahead and mark this as default then for assigned to so that if another dev sees it they'll take it over. Since it's a regression it'll get higher priority and hopefully be fixed by release.
Comment 7 Miklos Vajna 2013-01-10 10:52:52 UTC
Hard to see how this is a regression, as far as I see this is only triggered by the new commented text range feature. Anyway, I'm taking a look.
Comment 8 K Udo Schuermann 2013-01-10 14:47:16 UTC
@Miklos Vajna: Whether this is a regression is debatable, but it is a misfeature involving the (new ranged) comment. In any case, thanks for looking at this!

I have spent some hours digging through the code, but haven't managed to get my head around how it all hangs together. What I have come away with, however, is the sense that the marker at the front of the range is probably marked read-only, but this read-only state "bleeds" into the preceding character because some other part of the code (perhaps because of an error-by-one) mistakes the read-only state of the marker as applying to the preceding character.

I may be completely off, but maybe this helps. At least I hope it does not send you off on a wild goose chase.
Comment 9 Not Assigned 2013-01-10 15:24:19 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=fd0da52d371ca13d66be3b04d89f9d9cf73baddd

fdo#57938 SwPaM::HasReadonlySel: commented text ranges are not read-only



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.
Comment 10 Miklos Vajna 2013-01-10 15:40:36 UTC
Fixed in master, marking as resolved.

-4-0 review: https://gerrit.libreoffice.org/1629
Comment 11 Not Assigned 2013-01-11 11:05:10 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5a0dc1d1fdeac4b06b39ea616374d3f81b6be8a9&h=libreoffice-4-0

fdo#57938 SwPaM::HasReadonlySel: commented text ranges are not read-only


It will be available in LibreOffice 4.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.
Comment 12 K Udo Schuermann 2013-01-12 06:47:49 UTC
Test result: It is better now, but not fixed: A character located immediately before the ranged comment can be inserted (that's an improvement), but deleting a character immediately preceding the ranged comment still does not work (no matter if that character was present before the ranged comment was defined, or was inserted later).

How to reproduce:

Step 1-8 as in comment 2, then:
9. Press backspace

Expected behavior: Typed character is removed again.

Observed behavior: Character is not removed.

Also: If the character typed in step 8 (see comment 2) was a newline/Enter the backspace in step 9 causes the cursor to skip backwards over the newline; if it was a "normal" character, the cursor does not move at all. In either case the character immediately before the ranged comment is not removable (not with [Delete] key, or any other means) so long as the ranged comment persists.

Note: Inserting another character between the undeletable character and the start of the ranged comment will cause the newly inserted character, which is now the one immediately before the ranged comment, to become "stuck" (not deletable) instead.

Reopening as original bug report mentioned inability to delete, even if Steps to Reproduce were not as explicit.
Comment 13 Not Assigned 2013-01-14 13:41:11 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=50b6dc0099ff61050b82a2e37e70d643151e7ce7

fdo#57938 SwPaM::HasReadonlySel allow editing before commented text ranges



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.
Comment 14 Not Assigned 2013-01-14 14:39:14 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=bf670bcd5e60eae95a6bd297391799f13e55db9d

fdo#57938 testcase



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.
Comment 15 Miklos Vajna 2013-01-14 15:52:33 UTC
The char deletion problem is now resolved on master, -4-0 review: https://gerrit.libreoffice.org/1673
Comment 16 K Udo Schuermann 2013-01-14 17:22:00 UTC
Tested: Works as expected now.
Comment 17 Not Assigned 2013-01-14 18:57:43 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f39fa5aad48dc93bf24a328297635e5e46a65cff&h=libreoffice-4-0

fdo#57938 SwPaM::HasReadonlySel allow editing before commented text ranges


It will be available in LibreOffice 4.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.
Comment 18 Thomas van der Meulen [retired] 2013-08-22 06:11:53 UTC
it looks like bug 68404 looks some thing like this, could some one take a look pleass? 

Thank you in advance,
Thomas