Bug 97897 - Recalculate does not always work
Summary: Recalculate does not always work
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:5.2.0 target:5.1.2 target:5.0....
Keywords: bibisected, bisected, regression
: 98216 98486 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-02-16 10:58 UTC by Winfried Donkers
Modified: 2018-09-01 10:45 UTC (History)
10 users (show)

See Also:
Crash report or crash signature:


Attachments
Calc Document with formula (8.73 KB, application/x-vnd.oasis.opendocument.spreadsheet)
2016-02-16 10:58 UTC, Winfried Donkers
Details
Calc document with formulas (8.58 KB, application/vnd.oasis.opendocument.spreadsheet)
2016-02-17 08:10 UTC, Winfried Donkers
Details
Calc document with formulas (15.12 KB, application/vnd.oasis.opendocument.spreadsheet)
2016-12-29 23:23 UTC, Lorenzo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Winfried Donkers 2016-02-16 10:58:32 UTC
Created attachment 122676 [details]
Calc Document with formula

Steps to reproduce:
1. open attachment
2. select cell B3
3. press F9 or Extra-Cell-Recalculate
4. expected behaviour: the result in B3 changes from 3 to 4
   actual behaviour: formula is not recalculated.

Only when cell A1 or A2 is changed, F9 functions.

Ctrl-Shift-F9 dos function in all cases, problem only exists for F9.

Reproduced in versions 4.4.7, 5.0.4, 5.0.5 and 5.2 (master) on Windows and on Linux.

The current behaviour is confusing and leads to wrong user-interpretations of formula results.
Comment 1 Cor Nouws 2016-02-16 15:14:18 UTC
Hi Winfried,

(In reply to Winfried Donkers from comment #0)

> The current behaviour is confusing and leads to wrong user-interpretations
> of formula results.

Thanks for filing. I think you make a valid point here. How to explain that to random users, that you don't have contact with?

Cheers,
Cor
Comment 2 Cor Nouws 2016-02-16 15:15:42 UTC
should we mark this as a regression, or is it collateral-result/damage caused by other much wanted improvements (in which case marking as regression sometimes is considered to be a bit problematic..) ?
Comment 3 Winfried Donkers 2016-02-16 15:40:50 UTC
(In reply to Cor Nouws from comment #2)
> should we mark this as a regression [...] ?

I don't know if the behaviour was different in earlier versions (normally I don't use manual calculation, but this time a colleague mentioned a 'bug' (wrong result for complex formula) to me.)

I haven't found the code that is called by F9, so I can't see yet whether the current behaviour is intentional. In which case I would like to propose a Shift-F9 (or something) to force calculation of the cell.
You don't want to use Ctrl-Shift F9 in a spreadsheet document with hundreds of nested VLOOKUP formulas ;-)
Comment 4 Cor Nouws 2016-02-16 16:14:52 UTC
(In reply to Winfried Donkers from comment #3)
> (In reply to Cor Nouws from comment #2)
> > should we mark this as a regression [...] ?
> 
> I don't know if the behaviour was different in earlier versions

In 3.6.6.1 - just picked that one - simply F9 does work
Comment 5 Timur 2016-02-16 17:34:15 UTC
Regression from 4.0, tested with beta. 3.6 works fine with F9.
Comment 6 Winfried Donkers 2016-02-17 08:10:35 UTC
Created attachment 122714 [details]
Calc document with formulas

It's worse:
-open second attachment (Calc document with autocalculate ON)
-delete cell A4
-expected behaviour: cells A5:A8 showing revised results
 actual behaviour: cells A5:A8 do not change

(Possibly this behaviour is unrelated, but as I haven't found the code where recalculate is supposed to happen, I can't tell yet.)
Comment 7 Winfried Donkers 2016-02-17 08:28:06 UTC
(In reply to Winfried Donkers from comment #6)
> Created attachment 122714 [details]
> Calc document with formulas
> 
> It's worse:
> -open second attachment (Calc document with autocalculate ON)
> -delete cell A4
> -expected behaviour: cells A5:A8 showing revised results
>  actual behaviour: cells A5:A8 do not change
> 
> (Possibly this behaviour is unrelated, but as I haven't found the code where
> recalculate is supposed to happen, I can't tell yet.)

Pressing F9 in one of the cells A5:A8 doesn't help either, Ctrl-Shift-F9 is needed to recalculate.
Comment 8 raal 2016-02-18 19:58:20 UTC
bisected file test2.ods

d12d1a450eb7599eb60ba605ce7a7e8b12bcb744 is the first bad commit
commit d12d1a450eb7599eb60ba605ce7a7e8b12bcb744
Author: Matthew Francis <mjay.francis@gmail.com>
Date:   Sun Mar 15 06:07:23 2015 +0800

    source-hash-5ef856e975d7c0396984d588a43fd1a7c7085c55
    
    (Bibisect: Skipped preceding irrelevant commit(s) b2efe90c07baa45d2ee7e13138dd306a29738473)
    
    commit 5ef856e975d7c0396984d588a43fd1a7c7085c55
    Author:     Kohei Yoshida <kohei.yoshida@collabora.com>
    AuthorDate: Tue Nov 18 21:03:05 2014 -0500
    Commit:     Kohei Yoshida <kohei.yoshida@collabora.com>
    CommitDate: Tue Nov 18 21:03:50 2014 -0500
    
        Use group area listeners during ODS import.
    
        Change-Id: Id01f9021dda7f33255f8206174cd730507ab55ad

:040000 040000 126e010c62ce770ece406648bd72b0d2505c593f e7bee6d7439acbe7be3fd506baefed81c493b191 M	opt
Comment 9 Winfried Donkers 2016-02-19 14:55:16 UTC
@Kohei:
In commit 5ef856e975d7c0396984d588a43fd1a7c7085c55 (19 Nov 2014)
you removed 2 calls to   
       StartListeningTo( pDocument );

Putting these back in makes the problems with https://bugs.documentfoundation.org/attachment.cgi?id=122714 disappear.

Do you remember anything about this commit and if so, could you tell me about the why? That way I can try to fix the problem without breaking unknown matters.
Should you wish to do it yourself, I won't feel offended ;-)

BTW, this doesn't fix the problem of https://bugs.documentfoundation.org/attachment.cgi?id=122676 .
Comment 10 Winfried Donkers 2016-02-26 06:49:43 UTC
Working on a quick patch for the second problem.
It is caused by commit 5ef856e975d7c0396984d588a43fd1a7c7085c55 and reverting one removed call fixes the problem. 
But as the above commit aimed to improve performance, it is not the best fix. That however may prove too complex to find in a short time. Probably better to fix now for versions 5.0 and 5.1 and try that best fix next...
Comment 11 raal 2016-02-26 18:59:04 UTC
(In reply to Winfried Donkers from comment #0)
> Created attachment 122676 [details]
> Calc Document with formula
> 
> Steps to reproduce:
> 1. open attachment
> 2. select cell B3
> 3. press F9 or Extra-Cell-Recalculate
> 4. expected behaviour: the result in B3 changes from 3 to 4
>    actual behaviour: formula is not recalculated.
> 
> Only when cell A1 or A2 is changed, F9 functions.
> 
> Ctrl-Shift-F9 dos function in all cases, problem only exists for F9.
> 
> Reproduced in versions 4.4.7, 5.0.4, 5.0.5 and 5.2 (master) on Windows and
> on Linux.
> 
> The current behaviour is confusing and leads to wrong user-interpretations
> of formula results.

Winfried this is only one line commit..

f9bbb93682b5028ef66fbf65a0e93aeefc1482ec is the first bad commit
commit f9bbb93682b5028ef66fbf65a0e93aeefc1482ec
Author: Matthew Francis <mjay.francis@gmail.com>
Date:   Thu May 28 18:33:32 2015 +0800

    source-hash-f571104ef38ba9f7f6073e22c2374add7aa73887
    
    commit f571104ef38ba9f7f6073e22c2374add7aa73887
    Author:     Kohei Yoshida <kohei.yoshida@collabora.com>
    AuthorDate: Mon Jan 27 19:08:30 2014 -0500
    Commit:     Kohei Yoshida <kohei.yoshida@collabora.com>
    CommitDate: Mon Jan 27 19:11:55 2014 -0500
    
        fdo#69244: Avoid putting these cells in formula tree prematurely.
    
        Marking the cell dirty alone appears to be sufficient to trigger resetting
        of number format.  SetDirty() would mark it dirty *and* put the cell into
        formula tree, which would prevent proper value propagation as seen in the
        bug report.
    
        Change-Id: Ie68f996112938fe286a9bd50c38404f9df6f4ca1
Comment 12 Cor Nouws 2016-02-27 10:37:42 UTC
*** Bug 98216 has been marked as a duplicate of this bug. ***
Comment 13 Winfried Donkers 2016-02-29 11:35:28 UTC
(In reply to raal from comment #11)
>     commit f571104ef38ba9f7f6073e22c2374add7aa73887

Reverting this commit makes F9 work again with attachment 122676 [details] .
I can't see the unwanted consequences of just reverting this commit yet, so I'm not yet at the proper solution.
Comment 14 Commit Notification 2016-02-29 17:00:59 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

fix the "group not recalculated after delete" second part of tdf#97897

It will be available in 5.2.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 15 Eike Rathke 2016-02-29 17:56:40 UTC
Pending review
https://gerrit.libreoffice.org/22783 for 5-1
https://gerrit.libreoffice.org/22784 for 5-0
Comment 16 Eike Rathke 2016-02-29 18:18:53 UTC
For the first part of this bug, F9 not recalculating a loaded document when AutoCalc was/is off, there's not much we can do about other than forcing a full hard recalc the first time F9/recalc is executed or AutoCalc is switched on after load.

Other than that we could (should?) calculate dirty dependencies before saving such a document as to not save a dirty doc. Maybe only after having asked the user what s/he wants.
Comment 17 Winfried Donkers 2016-03-01 06:47:46 UTC
(In reply to Eike Rathke from comment #16)
> For the first part of this bug, F9 not recalculating a loaded document when
> AutoCalc was/is off, there's not much we can do about other than forcing a
> full hard recalc the first time F9/recalc is executed or AutoCalc is
> switched on after load.
> 
> Other than that we could (should?) calculate dirty dependencies before
> saving such a document as to not save a dirty doc. Maybe only after having
> asked the user what s/he wants.

IMHO it would be a good idea to warn the user when opening a document that has autocalculate off that the document may contain formulas that need recalculation.
Giving a choice to do a forced recalculation would make it even more user friendly. (Somewhat similar to the warning you get when a document contain macros.)

I know from experience in the company where I work that opening such a document leads to erroneous interpretation of the data and even to suspicion that Calc is no good.
Comment 18 Commit Notification 2016-03-01 11:39:10 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

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

fix the "group not recalculated after delete" second part of tdf#97897

It will be available in 5.1.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 19 Cor Nouws 2016-03-07 12:23:43 UTC
*** Bug 98486 has been marked as a duplicate of this bug. ***
Comment 20 Commit Notification 2016-03-11 22:57:12 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

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

tdf#97897 (first problem) fix for not recalculating on F9.

It will be available in 5.2.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 21 Commit Notification 2016-03-24 13:06:54 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

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

fix the "group not recalculated after delete" second part of tdf#97897

It will be available in 5.0.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.
Comment 22 Timur 2016-03-29 18:01:50 UTC
Looks like fixed, at least as reported in Comment 0.
Comment 23 Timur 2016-03-29 18:07:46 UTC
(In reply to Winfried Donkers from comment #6)
> Created attachment 122714 [details]
> 
> -open second attachment (Calc document with autocalculate ON)
> -delete cell A4
> -expected behaviour: cells A5:A8 showing revised results
>  actual behaviour: cells A5:A8 do not change

Now:
 -open attachment 122714 [details]
 -delete cell A4, cells A5:A8 show revised results (unlike before)
 -undo and than delete cell A4 again
  expected behaviour: cells A5:A8 show revised results
  actual behaviour: cells A5:A8 do not change
Comment 24 Winfried Donkers 2016-03-30 05:50:06 UTC
(In reply to Timur from comment #23)
> Now:
>  -open attachment 122714 [details]
>  -delete cell A4, cells A5:A8 show revised results (unlike before)
>  -undo and than delete cell A4 again
>   expected behaviour: cells A5:A8 show revised results
>   actual behaviour: cells A5:A8 do not change

Yes, I confirm. 
Moving the cursor (anywhere, including away from A4 and back to A4) between the first and the second delete action shows expected behaviour, though.  
As this regards the second patch of this bug report, I have put Eike as assignee.
Comment 25 Commit Notification 2016-04-06 11:49:00 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

(re)broadcast if value replaces cell of grouped formulas, tdf#97897 related

It will be available in 5.2.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 26 Eike Rathke 2016-04-06 12:02:10 UTC
That does not solve the problem of comment 23 but yet another related one..
Pending review
https://gerrit.libreoffice.org/23855 for 5-1
https://gerrit.libreoffice.org/23856 for 5-0
Comment 27 Commit Notification 2016-04-06 17:54:11 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

(re)broadcast, same as in ScDocument::SetString(), tdf#97897 related

It will be available in 5.2.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 28 Eike Rathke 2016-04-06 18:06:06 UTC
And another one..
Pending review
https://gerrit.libreoffice.org/23871 for 5-1
https://gerrit.libreoffice.org/23870 for 5-0
Comment 29 Commit Notification 2016-04-07 06:47:22 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=24635317f09ecf48e5d800147b6a2b95963b69a7&h=libreoffice-5-1

(re)broadcast, same as in ScDocument::SetString(), tdf#97897 related

It will be available in 5.1.3.

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 30 Commit Notification 2016-04-07 10:01:03 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#97897 (re)broadcast if formula groups were split

It will be available in 5.2.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 31 Eike Rathke 2016-04-07 10:36:04 UTC
Pending review
https://gerrit.libreoffice.org/23894 for 5-1
https://gerrit.libreoffice.org/23895 for 5-0
Comment 32 Commit Notification 2016-04-08 14:05:29 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

unit test for replacing fragments of shared formula groups, tdf#97897

It will be available in 5.2.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 33 Commit Notification 2016-04-12 14:11:04 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=95c9d125df7c1c3fe6400cf51f5b4ca9b91c8834&h=libreoffice-5-1

(re)broadcast if value replaces cell of grouped formulas, tdf#97897 related

It will be available in 5.1.3.

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 34 Commit Notification 2016-04-12 14:11:10 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1345cea38ecb51655375e77f5aa77aa9461d240b&h=libreoffice-5-1

Resolves: tdf#97897 (re)broadcast if formula groups were split

It will be available in 5.1.3.

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 35 Commit Notification 2016-04-12 14:11:14 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

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

(re)broadcast, same as in ScDocument::SetString(), tdf#97897 related

It will be available in 5.0.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.
Comment 36 Commit Notification 2016-04-12 14:11:18 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

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

(re)broadcast if value replaces cell of grouped formulas, tdf#97897 related

It will be available in 5.0.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.
Comment 37 Commit Notification 2016-04-12 14:11:22 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

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

Resolves: tdf#97897 (re)broadcast if formula groups were split

It will be available in 5.0.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.
Comment 38 Lorenzo 2016-12-29 23:23:21 UTC
Created attachment 130022 [details]
Calc document with formulas

Changing F20 between 3, 4 and 5 (for ex) should change results in cells E23:E40, but it doesn't (not all of them).
Comment 39 Lorenzo 2016-12-29 23:30:59 UTC
Hello,

   I think the problem is not solved. I attached a file https://bugs.documentfoundation.org/attachment.cgi?id=130022 where you can verify the problem. 
   If you change cell F20 form 3 to 4 (for example), cells in E23:E40 should refresh, but they do not refresh well. For example, cell E6 should show a number and it is still "blank". If are in this cell and go to the function wizard (the fx symbol, next to the summation one), you see that it actually can calculate the new result. Pressing F9 does nothing. 

I test this in Linux (Ubuntu 14.04), LO version: 5.2.3.2

Steps to reproduce:
1. open attachment
2. select cell F20 and change its value from 3 to 4.
3. expected behaviour: the result in E6 (and below it) changes from 'blank' to a number (123).
   actual behaviour: formula is not recalculated (it is still 'blank').
Comment 40 Xisco Faulí 2016-12-30 11:17:19 UTC
(In reply to Lorenzo from comment #39)
> Hello,
> 
>    I think the problem is not solved. I attached a file
> https://bugs.documentfoundation.org/attachment.cgi?id=130022 where you can
> verify the problem. 
>    If you change cell F20 form 3 to 4 (for example), cells in E23:E40 should
> refresh, but they do not refresh well. For example, cell E6 should show a
> number and it is still "blank". If are in this cell and go to the function
> wizard (the fx symbol, next to the summation one), you see that it actually
> can calculate the new result. Pressing F9 does nothing. 
> 
> I test this in Linux (Ubuntu 14.04), LO version: 5.2.3.2
> 
> Steps to reproduce:
> 1. open attachment
> 2. select cell F20 and change its value from 3 to 4.
> 3. expected behaviour: the result in E6 (and below it) changes from 'blank'
> to a number (123).
>    actual behaviour: formula is not recalculated (it is still 'blank').

Hi Lorenzo,
Could you please open a new follow-up ticket instead, and put a comment in here linking to it?
Regards