Bug 115530 - Conditional formatting isn't updated when formula is updated (XLSX)
Summary: Conditional formatting isn't updated when formula is updated (XLSX)
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
5.2.0.4 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.1.0 target:6.0.2 target:5.4.6
Keywords: bibisected, bisected, regression
Depends on:
Blocks: XLSX-Conditional-Formatting
  Show dependency treegraph
 
Reported: 2018-02-07 22:14 UTC by Aron Budea
Modified: 2018-02-21 17:04 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample XLSX (8.34 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2018-02-07 22:17 UTC, Aron Budea
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Budea 2018-02-07 22:14:13 UTC
- Open the attached XLSX file.
- Change A1 to 2.

A10:A11 should alternate background colors based on A1 being odd/even.

=> No changes shown in A10:A11.

- Scroll away and back.

=> Colors in A10:A11 are finally updated.

Observed using LO 6.0.0.3 & 5.2.0.4 / Windows 7.
No issue using LO 5.1.0.3.
=> regression

No issue when opening the same file saved as XLS or ODS.
Comment 1 Aron Budea 2018-02-07 22:17:17 UTC
Created attachment 139679 [details]
Sample XLSX
Comment 2 Aron Budea 2018-02-07 22:18:03 UTC
Bibisected to the following commit using repo bibisect-win32-5.2. Adding Cc: to Markus Mohrhard.

author		Markus Mohrhard <markus.mohrhard@googlemail.com>	2016-03-26 13:11:53 +0100
committer	Markus Mohrhard <markus.mohrhard@googlemail.com>	2016-03-26 16:59:20 +0100

switch to a listener based cond format update, tdf#95437
Comment 4 Aron Budea 2018-02-08 04:07:37 UTC
It seems that when ScConditionEntry::StartListening() is called during XLSX import, the ranges aren't there yet.

My idea is that in the following function:
void CondFormat::finalizeImport()
https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/condformatbuffer.cxx#1052

...move the following line:
mpFormat->SetRange(maModel.maRanges);

...so it precedes this one:
maRules.forEachMem( &CondFormatRule::finalizeImport );

Based on a quick test that seems to fix the issue, but not sure if there's more to it.
Comment 5 Markus Mohrhard 2018-02-17 21:04:39 UTC
(In reply to Aron Budea from comment #4)
> It seems that when ScConditionEntry::StartListening() is called during XLSX
> import, the ranges aren't there yet.
> 
> My idea is that in the following function:
> void CondFormat::finalizeImport()
> https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/
> condformatbuffer.cxx#1052
> 
> ...move the following line:
> mpFormat->SetRange(maModel.maRanges);
> 
> ...so it precedes this one:
> maRules.forEachMem( &CondFormatRule::finalizeImport );
> 
> Based on a quick test that seems to fix the issue, but not sure if there's
> more to it.

I think it should fix all cases. XLSX does not allow some of the complicated formulas that would require us to move the formula calculation after the import.
Comment 6 Commit Notification 2018-02-19 23:07:34 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "master":

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

for listeners the range needs to be set before the formula, tdf#115530

It will be available in 6.1.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 7 Commit Notification 2018-02-20 01:32:04 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2e06dfb831a93b8207cc4e0f43221ef5eeb1853b&h=libreoffice-6-0

for listeners the range needs to be set before the formula, tdf#115530

It will be available in 6.0.2.

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 8 Commit Notification 2018-02-20 02:47:27 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

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

add test for tdf#115530

It will be available in 6.1.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 9 Commit Notification 2018-02-21 17:04:42 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

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

for listeners the range needs to be set before the formula, tdf#115530

It will be available in 5.4.6.

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.