Bug 95295 - Calc is fragmenting ranges for conditional formats without need.
Summary: Calc is fragmenting ranges for conditional formats without need.
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.6.5.2 release
Hardware: All All
: low minor
Assignee: Not Assigned
URL:
Whiteboard: target:6.1.0
Keywords:
Depends on:
Blocks: Conditional-Formatting
  Show dependency treegraph
 
Reported: 2015-10-24 11:45 UTC by Wolfgang Jäger
Modified: 2023-02-17 21:07 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
The virgin version (18.87 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-10-24 11:45 UTC, Wolfgang Jäger
Details
After Copy/Past with LibO (29.74 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-10-24 11:46 UTC, Wolfgang Jäger
Details
After Copy/Paste with AOO (31.73 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-10-24 11:46 UTC, Wolfgang Jäger
Details
A new test file with a newer Calc (54.81 KB, application/vnd.oasis.opendocument.spreadsheet)
2017-04-09 12:10 UTC, Jouni Järvinen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Wolfgang Jäger 2015-10-24 11:45:11 UTC
Created attachment 119922 [details]
The virgin version

The fragmenting itself is notorious. I will focus on the "without need" here.

Reproducing my observations:

1. Create a new Calc document, 1 sheet.
2. Create values for A1:D10 and define a conditional format for this range.
You may use the attached CFfragmentingDemo001.ods instead.
3. Inspect the CF via >'Format'>'Conditional Formatting'>'Manage...'
4. Verify that there is exactly one range: A1:D10.
3. Save the document (001).

6. Select B3 and 'Copy' (Ctrl+C). 
7. Go to C6 and 'Paste' (Ctrl+V, accept overwriting).
8. Verify that there are now 2 ranges one of which is composed of 4 parts: A1:D5;A7:D10;A6:B6;D6. The other one is C6 (single cell).
9. Save AS... file variant 002LibO. I did respectively and attach the file. 


10. Close the 002LibO file and shut down LibreOffice.
11. Start AOO 4.1.1 and open the 001 file with it.
12. Repeat steps 6. and 7. from above.
13. Save AS... file variant 002AOO.

14. Close AOO and open 002AOO with LibreOffice Calc.
15. Inspect the CF and verify that there is still only one, the original, range.

16. Agree that the fragmentation by LibO Calc was "without need".

I would treat this as a bug. You may handle it as a feature request.
Comment 1 Wolfgang Jäger 2015-10-24 11:46:07 UTC
Created attachment 119923 [details]
After Copy/Past with LibO
Comment 2 Wolfgang Jäger 2015-10-24 11:46:47 UTC
Created attachment 119924 [details]
After Copy/Paste with AOO
Comment 3 Wolfgang Jäger 2015-10-24 11:55:40 UTC
As I mentioned AOO4.1.1 as the actually used version from the other branch, I should also post that the example was created with LibO 5.0.2.
(Additional versions tested beginning with 3.6.5)
Comment 4 m_a_riosv 2015-10-24 13:13:57 UTC
Hi Waolfgan, thanks for reporting.

There is a request for enhancement for at least an option to reunify the ranges.

please post there your comment.

*** This bug has been marked as a duplicate of bug 87274 ***
Comment 5 Wolfgang Jäger 2015-10-24 16:02:05 UTC
I knew the bug 87274 and had added me to its CC list.
I do not regard this bug as a duplicate of it.

The other bug is much more complicated:
1. In its demanding a new feature never before implemented
2. In reporting crashes the condition of which I did not actually understand. (I also had CF crashes with earlier versions after copying sheets containing CF. I didn't report that because I couldn't always clearly reproduce it, and because my observations were too complicated.)

This bug IS a bug and also a REGRESSION. The wrong (though not destructive) behaviour was not present in V3.3. and is not in AOO. It was introduced seemingly together with the CF manager but not depending on it. 

The bug is clearly described and can be fixed without introducing any new features. It is clearly distinguished from bug 87274 as fixing does not require any comparison of CF settings (for equivalence). On pasting a CF copied from another cell a check is necessary if both these cells belong to the same CF range. 

(This is obsolete, of course, if someone is working on a complete redesign CF management.)

Set Status to NEW again
Comment 6 m_a_riosv 2015-10-24 20:56:12 UTC
As you like, but please revert the status to UNCONFIRMED, it's not proper set up as new you own reports.
Comment 7 Wolfgang Jäger 2015-10-24 21:44:23 UTC
Sorry.
Set to UNCONFIRMED
Comment 8 Joel Madero 2015-10-30 04:10:05 UTC
I really don't understand how this isn't a duplicate of that other issue but to avoid an unnecessary flame war.


Bodhi 3.x
LibreOffice 5.0.2.2


NEW
Minor - can slow down but won't prevent high quality work;
Low - default for minor bugs

Fixing the issue as described in bug #87274 would of course fix this issue as well.
Comment 9 Wolfgang Jäger 2015-10-30 12:26:13 UTC
(In reply to Joel Madero from comment #8)
> I really don't understand how this isn't a duplicate of that other issue but
> to avoid an unnecessary flame war.
> 
Sorry! No flame war intended!
If someone wants to mark this bug as a duplicate again I will keep silent. But-

- I was not clear enough:
1. The first paragraph of the description of bug 87274 by "m.a.riosv" is actually concerning exactly the same issue. 
2. The attachment created there to explain things in more detail is not working.
3. The subsequent two paragraphs make things ambiguous, and finally shift to an enhancement request - as also "raal" saw it in his first comment.
4. The short description (subject) of that bug emphasises this request exclusively. I was instructed that it is not best practise to mix up reporting about an actual bug which is also a regression (legacy versions and AOO don't show it) and requesting a new (and valuable!) feature.
5. Therefore I decided to create a report about the "naked bug".

"...in the manage CF dialog to reunify ranges with the same CF.." requires to decide what is "same CF" and what not. Difficult!
Fixing this bug only requires to recognise "same range" as already is done by older versions and AOO. Easy!

Waiting for the implementation of "the new feature" will lead to nothing because this is a really complicated task, possibly depending on a fundamental redesign not only of the manager but of the CF itself, which might then be heavily incompatible with other software within the interoperability range. My bet: It will not be done within the next 10 years. 

If "same CF" in the other bug was intended to only mean "same range" this is misleading. The word "conditions" in the subject should exclude this interpretation, anyhow.
Comment 10 QA Administrators 2016-11-08 11:52:05 UTC Comment hidden (obsolete)
Comment 11 Jouni Järvinen 2017-03-05 18:53:21 UTC
LO 5.3.0.3 x64 on Win7 Ultimate SP1 x64. This problem exists like before, no improvement, not even in 5.2 series I had with the current usage of Calc.

You don't even need to do the repro as outlined in OP, you get further (word 'worsening' ?) fragmentation until the condition has been duplicated across every cell in the sheet, if your machine can handle such task and you have the time to wait.

Nowadays I tend to use a local install of LO Calc to make very heavily mathematical spreadsheets where conditional formatting is used to highlight smallest and largest values (sometimes even the middle value) across a column, or even across a certain range of cells across possibly more than 1 column: this fragmentation occurs every single time those cells with formulas are modified by cell copying handle or copypasting data into.
Comment 12 Jouni Järvinen 2017-04-09 12:08:41 UTC
Bumping cuz it's obvious this hasn't been looked into for one bit, especially the details. Wolfgang Jäger's email:
>>>>>>>>>>>>>>>>
A workaround?
- In the few cases I still intend to use CF to a larger extent, I develop the conditions in a spare range commenting on the purpose and on the range(s) to which they should be applied finally.
- Having completed the functional design of the sheets and needing no further scaling or copy/paste operations (hopefully) I add the "highlighting" functionality in the very last step.
- Even if I decided to also manage CF from the beginning for specific reasons, I will clean it completely at one point of the development and reinstate it unfragmented from cell-formulae.
<<<<<<<<<<<<<<<<

And like already said, this ain't a duplicate of #87274, this is a bug or possibly a leak (myself, I doubt it's a leak although that idea is mine) whereas #87274 is a request for a feature to merge applicable ranges broken by this problem.

A comment at #87274 references #97956, a problem similar to this, but doesn't sound the same.

Attached a new ODS from him.
Comment 13 Jouni Järvinen 2017-04-09 12:10:43 UTC
Created attachment 132418 [details]
A new test file with a newer Calc
Comment 14 Wolfgang Jäger 2017-10-02 18:04:21 UTC
Once again this was mixed up with the (re-)unification enhancement bug #87274. 

Unification should be expected to be much more difficult than avoiding fragmentation! 

This is not about an enhancement, but about a REGRESSION!

To draw the fragmentation discussion back to this topic where it should continue, I include the recent post I made there also in this comment:

Quoting myself from bug #87274, comment #14:
""
Now I am talking only of the fragmentaion, not of the (re-) unification which would mostly not be needed if not first fragmentation took place.

The problem arose, as far as I can tell, at (about?) the same time as the CF manager was introduced. In advance it was not just "not visible". It was not existing at all - and it still does not exist in AOO up to V 4.3.1. (where the classical CF dialog allowing for 3 conditions is still used). 

To be clear about steps to get evidence for my claim: 
Create a spreadsheet document with freshly defined CF (more than one probably).
Save it.
Open it in AOO.
Copy/Paste around in a way you know would cause unnecessary fragmentation in LibO.
Save again.
Open in LibO.
Call the CF manager.
Verify: No unnecessary fragmentation.

Some developer should study the CF-"code before CF manager" and find in what way fragmentation was NEWLY introduced. Repair should be feasible then. 

Unification of "equal" CFs (how to define exactly?) can wait then.
Comment 15 Commit Notification 2017-12-11 22:30:40 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

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

tdf#95295: don't add duplicate conditional formats

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 16 Xavier Van Wijmeersch 2017-12-13 17:34:22 UTC
I did a test with 'The virgin version'
and its working. Dragging with the mouse from D1:D10 to I1:I10 and there is only the range that is changed. No extended range.

Version: 6.1.0.0.alpha0+
Build ID: 6ca6d6ac912588a8f62d7e6b668ebec333752ebc
CPU threads: 8; OS: Linux 4.9; UI render: default; VCL: kde4; 
Locale: nl-BE (en_US.UTF-8); Calc: group threaded
Comment 17 Mike Kaganski 2017-12-25 12:13:09 UTC
(In reply to Xavier Van Wijmeersch from comment #16)
> I did a test with 'The virgin version'
> and its working. Dragging with the mouse from D1:D10 to I1:I10 and there is
> only the range that is changed. No extended range.

Thank you for testing.
Could you please clarify if your check confirms it fixed, or not: I'm confused with "and there is only the range that is changed" (I take it that you see the range of existing format extended, not a new format created, which is the desired result AFAICT), followed by "No extended range" - possibly you meant "no new format for extended range created"?
Comment 18 Xavier Van Wijmeersch 2017-12-25 13:58:11 UTC
@Mike
Sorry for the bad explanation
Yes i mean "no new format for extended range created" and "I see the range of existing format extended, not a new format created, which is the desired result"


Best regards
Xavier
Comment 19 Mike Kaganski 2017-12-25 13:59:31 UTC
(In reply to Xavier Van Wijmeersch from comment #18)

Thanks! So - closing FIXED.
Comment 20 Roman Kuznetsov 2018-05-29 12:25:25 UTC
Verified, really fixed
Comment 21 Wolfgang Jäger 2018-06-15 23:03:28 UTC
May I assume that the patch not aimed at minimizing the number of subranges used to represent the resulting SheetCellRanges for an edited CF?  

An example to clarify what I mean: 
I made a CF for D9:G22.
I copied E12:E18.
I pasted this to the range starting at F20. 

I got the SheetCellRanges D9:G19;D20:E22;G20:G22;F20:F26 [A](4 ranges). 

I might have expected: D9:G22;F23:F26 [B](2 ranges).

Is there a clear concept in what way to describe a connected selection requiring more than one range to describe it? 

===
Selecting all the celle (in time as [B]describes gives:
D9:E22;F9:E26;G9:G22 [C] (3 ranges).
Changing the order of selection does not change the representation.  
===

Coming from experiences with CurrentSelection and with queryContentCells / queryEmptyCells I assumed a scheme making the results unambiguous and thus easily comparable. Using different schemes in different cases may induce new issues if comparisons are once needed. 

Isn't it feasible to analyze and represent a union created by CF editing in the same way as the query methods do (type [C]?
Comment 22 Mike Kaganski 2018-06-16 02:59:37 UTC
(In reply to Wolfgang Jäger from comment #21)

This might be a valid enhancement; but this is outside of the scope of this issue. Please file a new issue for this. (And unifying the code that joins ranges in different parts of Calc is The Good Thing(TM) :-) )