Bug 134426 - Changing Paragraph format, editing Text and changing back leaves edited text's format unchanged
Summary: Changing Paragraph format, editing Text and changing back leaves edited text'...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.3.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:7.3.0 target:7.2.3 target:7.1.7
Keywords: bibisected, bisected, regression
: 140521 144456 144638 145176 (view as bug list)
Depends on:
Blocks: Writer-Styles-Paragraph
  Show dependency treegraph
 
Reported: 2020-07-01 06:53 UTC by Georg Jansing
Modified: 2021-11-08 07:57 UTC (History)
12 users (show)

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


Attachments
Resulting ODT file (8.53 KB, application/vnd.oasis.opendocument.text)
2020-07-01 06:53 UTC, Georg Jansing
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Georg Jansing 2020-07-01 06:53:04 UTC
Description:
See below, hard to describe without telling the actual steps to reproduce.

Steps to Reproduce:
1. Create new document in Writer
2. Type "Hello World"
3. Open "Styles and Formatting" sidebar, "Paragraph Styles"
4. Doubleclick "Heading 1"
5. Doubleclick word "WOrld" and Change it to "Me"
6. Doubleclick "Default Style" in "Styles and Formatting" sidebar


Actual Results:
"Hello" changes back to default style, but "Me" stays in "Heading 1" formatting.

Expected Results:
Whole line changes back to default style.


Reproducible: Always


User Profile Reset: No



Additional Info:
Remark: moving the cursor usually changes the selection in "Styles and Formatting" sidebar to the corresponding paragraph style currently in use. When moving it to the Word "Me", the selection is still at "Default Style".

Tested on Libreoffice 6.3.x on Windows and Libreoffice 6.4.4.2 on Linux.
Comment 1 Georg Jansing 2020-07-01 06:53:43 UTC
Created attachment 162549 [details]
Resulting ODT file
Comment 2 Dieter 2020-10-09 17:45:22 UTC
I confirm it with

Version: 7.0.2.2 (x64)
Build ID: 8349ace3c3162073abd90d81fd06dcfb6b36b994
CPU threads: 4; OS: Windows 10.0 Build 19041; UI render: Skia/Raster; VCL: win
Locale: he-IL (de_DE); UI: en-GB
Calc: threaded

It seems, that the substitution is treated as direct formatting. Personally I won't understand it as direct formatting, if you change a word.

So I change status to NEW, but add also design-team for further input and decision.
Comment 3 Heiko Tietze 2020-10-12 10:11:51 UTC
Situation after changing "World" into "Me"

<text:h text:style-name="Heading_20_1" text:outline-level="1">Hello <text:span text:style-name="T2">Me</text:span></text:h>

  <style:style style:name="T2" style:family="text">
   <style:text-properties fo:color="#ff0000"...
  </style:style>

Smells like a bug. Or what do you think, Mike?
Comment 4 Mike Kaganski 2020-10-12 10:43:53 UTC
This is a bug and a regression: was OK in Version: 5.0.0.5 (x64)
Build ID: 1b1a90865e348b492231e1c451437d7a15bb262b
Locale: ru-RU (ru_RU)
Comment 5 Mike Kaganski 2020-10-12 13:07:35 UTC
Regression after https://git.libreoffice.org/core/+/6abed0ea006f3616e40faf2ae782cf64f8ac2914
Comment 6 Xisco Faulí 2020-11-05 09:36:27 UTC
Still reproducible in

Version: 7.1.0.0.alpha1+
Build ID: 9c8ed8c8526b9b696d0bf592eb7d963950f3cef4
CPU threads: 4; OS: Linux 5.7; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

after fix for bug 137532 regressed from same commit
Comment 7 Timur 2021-02-21 19:30:30 UTC
*** Bug 140521 has been marked as a duplicate of this bug. ***
Comment 8 Timur 2021-02-21 19:31:43 UTC
Fix seems worse than the original issue. I'm in favor of revert.
Comment 9 Telesto 2021-09-25 00:48:44 UTC
@Xisco
Any change to revert this?

Already multiple people in favor of this:
1) Comment 8
2) Same opinion (of Mike Kaganski): bug 144638 comment 11
Comment 10 David 2021-10-01 08:03:08 UTC
It seems like in the file 'master/sw/source/uibase/wrtsh/wrtsh1.cxx' that line #269 (SetAttrSet(aCharAttrSet, SetAttrMode::DEFAULT, &aPaM);) in the function 'bDeleted' should only be executed if there is no character style already set.
Comment 11 David 2021-10-01 20:56:15 UTC
It seems like the line #248 (GetPaMAttr(&aPaM, aCharAttrSet);) also needs to be executed only if no character styles are used, otherwise if a whole word is replaced it still won't use the automatic character style.
Comment 12 David 2021-10-01 23:55:43 UTC
I think comment 11 can be disregarded.  The issue mentioned in that comment seems to no longer be happening after another re-compile.  The problem line seems to be 'aCharAttrSet, SetAttrMode::DEFAULT, &aPaM);' in wrtsh1.cxx.
Comment 13 Mike Kaganski 2021-10-02 04:38:47 UTC
(In reply to David from comment #12)

David, it's great that you handle this.
Do you need assistance to provide your patch to gerrit?
Comment 14 David 2021-10-02 08:06:54 UTC
A programmer needs to determine how to handle this.  

A.) The problem line can be commented out. This would revert the program to behaving like before 6.3 until it's dealt with properly later.

B.) All the code associated with tdf#79717 could be removed (currently about 10 lines). Personally I never thought it was a bug to begin with.

C.) The problem line could be re-programmed to detect if a character style is used.  I don't know how to do that.
Comment 15 Justin L 2021-10-12 12:22:08 UTC
*** Bug 144456 has been marked as a duplicate of this bug. ***
Comment 16 Justin L 2021-10-12 13:27:05 UTC
(In reply to Timur from comment #8)
> Fix seems worse than the original issue. I'm in favor of revert.

Me too. David, are you interested in crafting the revert?  (Unit test, then 04bd1925706360414438b814046b543c5e317d0a, then 6abed0ea006f3616e40faf2ae782cf64f8ac2914 I assume.)
Comment 17 David 2021-10-12 15:55:29 UTC
I could send someone a modified file if I knew who to send it to.
Comment 18 Justin L 2021-10-12 16:55:00 UTC
(In reply to David from comment #17)
> I could send someone a modified file if I knew who to send it to.

You send it to Gerrit :-)
https://wiki.documentfoundation.org/Development/gerrit
Comment 19 Justin L 2021-10-13 12:47:05 UTC
mstahl could probably fix this properly within an hour. I've spent several hours looking for alternative functions that could snapshot the existing direct formatting and re-apply it, but I've come up empty. SW is just too complex for me.
Comment 20 Justin L 2021-10-13 19:14:27 UTC
revert proposed at http://gerrit.libreoffice.org/c/core/+/123566
Comment 21 Commit Notification 2021-10-14 06:46:10 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/036d45b602c2f554f9bcc21fecbc3e3ac5b834da

tdf#134426 tdf#138873 sw:  Revert "tdf#79717 ...

It will be available in 7.3.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 22 Commit Notification 2021-10-14 08:18:34 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/49e8691530ea931eeee74fd88727c2451e98a3bd

tdf#134426 tdf#138873 sw:  Revert "tdf#79717 ...

It will be available in 7.2.3.

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 23 David 2021-10-14 08:54:35 UTC
Thank-you, Justin!  Looks Good!
Comment 24 Commit Notification 2021-10-14 10:24:08 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/08ede5d51a33f83ededee8a03c10f9bb8244690e

tdf#134426 tdf#138873 sw:  Revert "tdf#79717 ...

It will be available in 7.1.7.

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 25 Mike Kaganski 2021-10-16 17:17:53 UTC
*** Bug 145176 has been marked as a duplicate of this bug. ***
Comment 26 Mike Kaganski 2021-10-16 17:18:00 UTC
*** Bug 144638 has been marked as a duplicate of this bug. ***
Comment 27 Dieter 2021-11-08 07:26:46 UTC
VERIFIED with

Version: 7.3.0.0.alpha1+ (x64) / LibreOffice Community
Build ID: 742b8befecbcfc0cfab87cfcd87c83b7d8ef32ab
CPU threads: 4; OS: Windows 10.0 Build 19043; UI render: Skia/Raster; VCL: win
Locale: de-DE (de_DE); UI: en-GB
Calc: CL