Bug 164712 - CharBackColor returns different value compared to previous releases
Summary: CharBackColor returns different value compared to previous releases
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
24.8.4.2 release
Hardware: All All
: medium normal
Assignee: Armin Le Grand (allotropia)
URL:
Whiteboard: target:25.8.0 target:25.2.2
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2025-01-14 21:19 UTC by Jordi
Modified: 2025-02-13 14:14 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Version 7's behavior (75.98 KB, image/png)
2025-01-14 21:19 UTC, Jordi
Details
Version 24's behavior (83.78 KB, image/png)
2025-01-14 21:20 UTC, Jordi
Details
Sample file (17.29 KB, application/vnd.oasis.opendocument.text)
2025-01-16 09:50 UTC, Jordi
Details
Screnshoot, properties with 7.6.7 (97.48 KB, image/png)
2025-01-16 11:53 UTC, m_a_riosv
Details
Screnshoot, properties with 24.8.4 (26.67 KB, image/png)
2025-01-16 11:55 UTC, m_a_riosv
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jordi 2025-01-14 21:19:46 UTC
Created attachment 198545 [details]
Version 7's behavior

When testing a textcursor's CharBackColor property of a block that has multiple colors, the returned value is different between v7.6.7.2 and v24.8.4.2

Test envs,

Version: 24.8.4.2 (X86_64) / LibreOffice Community
Build ID: bb3cfa12c7b1bf994ecc5649a80400d06cd71002
CPU threads: 12; OS: Windows 10 X86_64 (10.0 build 19045); UI render: Skia/Raster; VCL: win
Locale: en-AU (en_AU); UI: en-GB
Calc: CL threaded

Version: 7.6.7.2 (X86_64) / LibreOffice Community
Build ID: dd47e4b30cb7dab30588d6c79c651f218165e3c5
CPU threads: 12; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: en-AU (en_AU); UI: en-GB
Calc: CL threaded


In v7 the result is Empty, while in v24 the result is one of the colors used.

Which is the correct behavior? I've used v7's Empty as a way to find multicolored paragraphs but v24 breaks this.
Comment 1 Jordi 2025-01-14 21:20:17 UTC
Created attachment 198546 [details]
Version 24's behavior
Comment 2 m_a_riosv 2025-01-15 22:46:28 UTC
Please attach a sample file.
Comment 3 Jordi 2025-01-16 09:50:18 UTC
Created attachment 198575 [details]
Sample file
Comment 4 m_a_riosv 2025-01-16 11:53:30 UTC
Created attachment 198576 [details]
Screnshoot, properties with 7.6.7
Comment 5 m_a_riosv 2025-01-16 11:55:28 UTC
Created attachment 198577 [details]
Screnshoot, properties with 24.8.4

Both with macros disable.
Comment 6 Jordi 2025-01-16 13:03:33 UTC
(In reply to m_a_riosv from comment #5)
> Created attachment 198577 [details]
> Screnshoot, properties with 24.8.4
> 
> Both with macros disable.

I think you uploaded the wrong screenshot...
Comment 7 raal 2025-01-16 16:16:39 UTC
Confirm with Version: 25.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: db6a6efdc49df054826e9019ebe955282de929a7
CPU threads: 4; OS: Linux 6.8; UI render: default; VCL: gtk3
Locale: he-IL (cs_CZ.UTF-8); UI: en-US
Calc: threaded
Comment 8 raal 2025-01-16 16:44:12 UTC
This seems to have begun at the below commit in bibisect repository/OS linux-64-24.2.
Adding Cc: to Armin Le Grand ; Could you possibly take a look at this one?
Thanks
 04d692d92e3fef4632e7cdabe449363eb1690381 is the first bad commit
commit 04d692d92e3fef4632e7cdabe449363eb1690381
Author: Jenkins Build User <tdf@pollux.tdf>
Date:   Fri Sep 8 18:01:49 2023 +0200

    source c1f3b34f871d2a6bb9ee7b912492be1164eec96f

156306: ITEM: preparations for more/easier changes II | https://gerrit.libreoffice.org/c/core/+/156306
Comment 9 Armin Le Grand (allotropia) 2025-01-31 15:42:54 UTC
To check and see differences I have built the last version before tdf#164712 bibisect'ed which is 93aa47535267435fd327368875e46a33f86b39d6. I *can* see differences in the display when the cursor is on line 3 ("CPU threads...").

In 'StyleInspector/ParagraphStyles/DefaultParagraphStyle/CharBackColor':
Before: 0x-1 grayed out (what probably means 'not set'?)
Current: 0x-1 but not grayed out

In 'StyleInspector/ParagraphStyles/ParagraphDirectFormatting':
Before:
- CharBackColor/CharBackTransparent/CharShadingValue grayed out
Current:
- CharBackColor/CharBackTransparent/CharShadingValue not grayed out

All values are the same, thus the differene seems to be the grayed-out state. I am not a Writer specialist and not sure what that means exactly, thus adding a specialist.
Comment 10 Armin Le Grand (allotropia) 2025-01-31 15:46:23 UTC
@michael.stahl@allotropia.de:
I need infos from you, I do not know enough about SW stuff here. See comment 9 above describing the differences when on line 3 of the bugdoc from comment 3.

What does 'grayed out' or 'not grayed out' in StyleInspector exactly mean? Does it *have* to be grayed out? Is it an error...?

Thanks in advance!
Comment 11 Michael Stahl (allotropia) 2025-01-31 17:40:06 UTC
it sounds like the properties now have the state PropertyState_DIRECT_VALUE and previously it was PropertyState_AMBIGUOUS_VALUE or so?

looks like the inspector panel is in sw/source/uibase/sidebar/WriterInspectorTextPanel.cxx and there is one function that sets the svx::sidebar::TreeNode::isGrey member but i dont know anything about this code.
Comment 12 Armin Le Grand (allotropia) 2025-02-03 11:21:10 UTC
Checked now at PropertyToTreeNode in sw/source/uibase/sidebar/WriterInspectorTextPanel.cxx in 'if (rIsGrey)' with moving cursor from VCL to CPU line.

Old version: true for
- "CharBackColor" x2
- "CharBackTransparent" x2
- "CharBackgroundComplexColor" x2
- "CharShadingValue" x2
- "CharLocale"

Master (new) version:
*exactly* the same - so I do not really understand (yet) why the display is different (?)
Comment 13 Armin Le Grand (allotropia) 2025-02-04 10:27:29 UTC
Checked in InsertValues the PropertyState that gets fetched using xPropertiesState->getPropertyState, especially for PropertyState_AMBIGUOUS_VALUE when changing from line VCL to GPU:

InsertValues:
InsertValues:
Property FillBitmapMode :PropertyState_AMBIGUOUS_VALUE
InsertValues:
Property Category :PropertyState_AMBIGUOUS_VALUE
Property DisplayName :PropertyState_AMBIGUOUS_VALUE
Property FillBitmapMode :PropertyState_AMBIGUOUS_VALUE
Property Hidden :PropertyState_AMBIGUOUS_VALUE
Property IsAutoUpdate :PropertyState_AMBIGUOUS_VALUE
Property IsPhysical :PropertyState_AMBIGUOUS_VALUE
Property StyleInteropGrabBag :PropertyState_AMBIGUOUS_VALUE

...but this is the *same* in old and new (?)
Also for PropertyState_DIRECT_VALUE, also identical...

I also checked all, but only difference is that there are new values added in newer version, all on PropertyState_DEFAULT_VALUE:

Property ParaFirstLineIndentUnit :PropertyState_DEFAULT_VALUE
Property ParaHyphenationCompoundMinLeadingChars :PropertyState_DEFAULT_VALUE
Property ParaHyphenationKeep :PropertyState_DEFAULT_VALUE
Property ParaHyphenationKeepType :PropertyState_DEFAULT_VALUE
Property ParaLeftMarginUnit :PropertyState_DEFAULT_VALUE
Property ParaRightMarginUnit :PropertyState_DEFAULT_VALUE

Thus: I cannot find a difference in PropertyStates...?
I already checked if 'isGrey' *does* actually change that gray state by deactivating it temporarily - it does...

This seems strange, but I see no change in PropertyStates, so where do the changes come from...?
Comment 14 Armin Le Grand (allotropia) 2025-02-04 13:00:59 UTC
I still do not uhhderstand why in old version the gray is shown in the inspector panel and not in new version. I *checked* now that rCurrent.isGrey in FillBox_Impl *does* have true and false values, even the same in both versions -> no idea. Also checked usage of weld::TreeView& rListBoxStyles in both versions, no difference(s) found...
Comment 15 Armin Le Grand (allotropia) 2025-02-05 10:25:30 UTC
I have now a BP in sw/source/uibase/sidebar/WriterInspectorTextPanel.cxx in both versions. Open testfile, klick on Line 'CPU...' between 'O' and 'S' of 'OS'. In debugger, the values for aParaDFNode (as example for CharBackColor here) are:

master: isGrey = true
old: isGrey = true

Thus SAME values coming out of UpdateTree method in sw/source/uibase/sidebar/WriterInspectorTextPanel.cxx. Thus the Item and ItemState accesses in that method did deliver identical results.

I also see in the panel right that in old it gets grayed out, but in master not. It looks as if *after* that method the entries in aParaDFNode get manipulated...?
Comment 16 Armin Le Grand (allotropia) 2025-02-05 11:38:06 UTC
I have now traced the values for 'Char Back Color' in FillBox_Impl where the isGrey is used. There are three entries when clicking at space between 'O' and 'S' in 'OS'. These are:

For old version:
Char Back Color.isGrey == 1 "0x-1"
Char Back Color.isGrey == 1 "0xc9211e"
Char Back Color.isGrey == 0 "0x66ff66"

For master:
Char Back Color.isGrey == 1 "0x-1"
Char Back Color.isGrey == 1 "0xc9211e"
Char Back Color.isGrey == 0 "0x66ff66"

Thus *identical* -> I hve no idea why
    rListBoxStyles.set_sensitive(*pResult, !rCurrent.isGrey, 0);
does not lead to grayed out entries in the list then.

@Caolan: Did something change for 'set_sensitive' that it does not show grayed-out entries anymore...?

@miguelangelrv@libreoffice.org, @bugs.df.org@myhideout.online
Is there a script that I can execute that shows the difference you describe? Please give a detailed, step-by-step explanation how to achieve that. Maybe I am too bad in Writer to get that. E.g. please explain how to bring up that dialog from the screenshots and where the cursor has to be placed when doing so. Thanks in advance!
Comment 17 Jordi 2025-02-05 16:08:29 UTC
(In reply to Armin Le Grand (allotropia) from comment #16)
> I have now traced the values for 'Char Back Color' in FillBox_Impl where the
> isGrey is used. There are three entries when clicking at space between 'O'
> and 'S' in 'OS'. These are:
> 
> For old version:
> Char Back Color.isGrey == 1 "0x-1"
> Char Back Color.isGrey == 1 "0xc9211e"
> Char Back Color.isGrey == 0 "0x66ff66"
> 
> For master:
> Char Back Color.isGrey == 1 "0x-1"
> Char Back Color.isGrey == 1 "0xc9211e"
> Char Back Color.isGrey == 0 "0x66ff66"
> 
> Thus *identical* -> I hve no idea why
>     rListBoxStyles.set_sensitive(*pResult, !rCurrent.isGrey, 0);
> does not lead to grayed out entries in the list then.
> 
> @Caolan: Did something change for 'set_sensitive' that it does not show
> grayed-out entries anymore...?
> 
> @miguelangelrv@libreoffice.org, @bugs.df.org@myhideout.online
> Is there a script that I can execute that shows the difference you describe?
> Please give a detailed, step-by-step explanation how to achieve that. Maybe
> I am too bad in Writer to get that. E.g. please explain how to bring up that
> dialog from the screenshots and where the cursor has to be placed when doing
> so. Thanks in advance!

Armin, download the sample file and run the macro in the doc called 'gotoLastUncheckedPar'.
Comment 18 Armin Le Grand (allotropia) 2025-02-05 18:03:22 UTC
Hi Jordi - thanks, that till help! There are sooo many macros in that file...
Comment 19 Jordi 2025-02-05 20:26:03 UTC
(In reply to Armin Le Grand (allotropia) from comment #18)
> Hi Jordi - thanks, that till help! There are sooo many macros in that file...

You are probably looking at the wrong place. The document itself only has three (3) macros, 'gotoLastUncheckedPar' is one of them.
Comment 20 Armin Le Grand (allotropia) 2025-02-06 10:03:10 UTC
Hi Jordi, you are probably right, that's why I asked in the end. I do not do much with macros, so for me it's not obvious which one to call or use. To avoid please just mention it next time. Regina usually adds a button to the bugdoc that you can press to execute the macro, that's really handy. I see a diff now, on it...
Comment 21 Armin Le Grand (allotropia) 2025-02-06 17:53:00 UTC
Made some progress, 1st stripped the basic script to directly create the problem without having to answer three more dialogs, that also makes debugging easier. I added
    if (rPropertyName == OUString("CharBackColor"))
to be able to break there. The 2nd call is the problematic.
SW merges all attributes for the text segments in SwTextNode::GetParaAttr in a complicated way. All seems to happen the same, but at the end when the result is applied to aFormatSet using
            aFormatSet.Differentiate( rSet );
whereby in rSet [21] is set correctly to invalid, it is set to
- nullptr in the old version (Item deleted from set)
- stays at the existing item in the new version.

That means the error is in SfxItemSet::Differentiate which I indeed had to adapt when doing the item changes. The old version was hard to understand, see my comments in new version:

    // CAUTION: In the former impl, the
    // - version for different ranges checked for SfxItemState::SET
    //   in rSet
    // - version for same ranges checked for
    //   nullptr != local && nullptr != rSet.
    // All together I think also using the text
    // "Delete all Items contained in rSet" leads to
    // locally delete all Items that *are *not* set in rSet
    // -> ==SfxItemState::SET

And here we seem to have a case that needs to act differently for SfxItemState::DISABLED. Have to check tomorrow.

NOTE: SfxItemSet::Intersect is *very* similar to SfxItemSet::Differentiate, so that one might be affected, too.
Comment 22 Armin Le Grand (allotropia) 2025-02-07 14:57:11 UTC
Corrected that now, see https://gerrit.libreoffice.org/c/core/+/181258
Comment 23 Commit Notification 2025-02-10 10:04:41 UTC
Armin Le Grand (Collabora) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/95d640e4c228181485af50248d90ad94228de9a4

tdf#164712: ITEM regression: Fix SfxItemSet::Differentiate

It will be available in 25.8.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 24 Commit Notification 2025-02-10 15:24:37 UTC
Armin Le Grand (Collabora) committed a patch related to this issue.
It has been pushed to "libreoffice-25-2":

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

tdf#164712: ITEM regression: Fix SfxItemSet::Differentiate

It will be available in 25.2.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 25 Commit Notification 2025-02-11 00:06:06 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/089e8d0ae15da9632456bdf0c59a3531a6853dee

tdf#164712: sw_odfexport: Add unittest

It will be available in 25.8.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.