Issue 122927 - =IF(AK3=" ";0;IF(AK3="Y";195;IF(AK3="N";125))) produces FALSE instead of $0.00
Summary: =IF(AK3=" ";0;IF(AK3="Y";195;IF(AK3="N";125))) produces FALSE instead of $0.00
Status: CLOSED FIXED
Alias: None
Product: Calc
Classification: Application
Component: configuration (show other issues)
Version: 4.0.0
Hardware: PC Windows 7
: P3 Major (vote)
Target Milestone: 4.0.1
Assignee: Clarence GUO
QA Contact:
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2013-08-01 12:40 UTC by Bill Watson
Modified: 2017-05-20 10:34 UTC (History)
8 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---
jsc: 4.0.1_release_blocker+


Attachments
sample spreadsheet by email (20.38 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-08-02 05:59 UTC, Edwin Sharp
no flags Details
Fix patch (1.83 KB, patch)
2013-09-06 10:03 UTC, Clarence GUO
clarence.guo.bj: review?
Details | Diff
proposed solution based on comment 19 (2.84 KB, patch)
2013-09-10 15:05 UTC, Oliver-Rainer Wittmann
no flags Details | Diff
constants and TRUE() and FALSE() with different ways for formatting (14.79 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-09-10 20:06 UTC, Regina Henschel
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description Bill Watson 2013-08-01 12:40:13 UTC
this formula in the previous version produced a result of $0.00 which is desired.
in the new 4.0 version it produces FALSE. not a big issue but had rather have the $0.00 answer instead. is there a work around for this?
Comment 1 Edwin Sharp 2013-08-01 15:50:44 UTC
Please attach spreadsheet.
Comment 2 Edwin Sharp 2013-08-01 19:13:46 UTC
Thank you.

$0.00 in 2.4.3
FALSE in Rev. 1503704.
Comment 3 Rob Weir 2013-08-01 20:10:13 UTC
What is the contents of cell AK3 in your spreadsheet?
Comment 4 Rob Weir 2013-08-01 21:30:18 UTC
Checked in Excel 2013 and they give the result as "FALSE".

So I'm not sure this is a bug, even if it is different than 3.4.1.  Better interop with Excel is a goal as well.
Comment 5 Edwin Sharp 2013-08-02 05:59:52 UTC
Created attachment 81234 [details]
sample spreadsheet by email

forgot to attach, sorry.
Comment 6 Edwin Sharp 2013-08-02 06:03:04 UTC
=IF(C3=" ";0;IF(C3="Y";195;IF(C3="N";125)))

C3 blank, so:

=IF(C3=" ";0;IF(C3="Y";195;FALSE))
=IF(C3=" ";0;FALSE)
=0

Excel is wrong.
Comment 7 Rob Weir 2013-08-07 18:07:29 UTC
C3 is empty, but the test is for a space, not an empty cell.  (It tests for " " not "").  Try entering a space in C3 and you'll get the 0.

What is happening here is the evaluation falls through to the embedded IF(C3="N";125).  Here the formula does not specify an "else" value.  So the correct behavior, per the ODF standard, is to return FALSE().  

See:

http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part2.html#__RefHeading__1018446_715980110


So I think this is correct.
Comment 8 Edwin Sharp 2013-08-07 18:52:19 UTC
But according to 5.14 "For calculation purposes, whitespace is ignored unless it is inside the contents of string constants or text surrounded by single quotes."?

On the other hand, Calc distinguishes between whitespaces i.e
A1 = one space
B1 = two spaces
C1 = if(A1=B1;"Equal";"Not equal") returns Not equal, although cells visually the same (blank).

IMHO fallible for user.
Comment 9 Rob Weir 2013-08-07 19:06:04 UTC
That section is talking about whitespace within a formula, not within cell content.

In any case, FALSE is the same as 0, like TRUE is 1, so the issue is one of formatting, not of the return value.  If the user wants to see the return value as 0, then this will occur if the inner condition is written as: IF(C3="N";125;0).
Comment 10 Edwin Sharp 2013-08-07 19:21:13 UTC
=IF(C3=" ";0;IF(C3="Y";195;IF(C3="N";125;0)))

=IF(C3=" ";0;IF(C3="Y";195;0))

=IF(C3=" ";0;0)

ThenValue = OtherwiseValue -> Test is meaningless.
Comment 11 Rob Weir 2013-08-08 02:17:49 UTC
I'll give you an ever better way of doing this, with no IF() calls at all.

How about this:   =195*(C3="Y")+125*(C3="N")

It also handles blanks and spaces, returning 0 in those cases.
Comment 12 Edwin Sharp 2013-08-08 07:19:17 UTC
Fidelity to our previous versions is much more important than compatibility to third party software.
Comment 13 Oliver-Rainer Wittmann 2013-08-12 08:12:50 UTC
(In reply to Edwin Sharp from comment #12)
> Fidelity to our previous versions is much more important than compatibility
> to third party software.

What is more important is hard to say from my point of view.
In general you want both which is very hard to achieve.
Comment 14 Oliver Brinzing 2013-08-15 14:17:58 UTC
.
Comment 15 Clarence GUO 2013-09-06 10:03:37 UTC
Created attachment 81470 [details]
Fix patch

In 121126, although in description it only mentioned format code issue, actually the fix is not only for format code but also for output string for logic formula cell. The second one introduced this issue.
I think the change for output string change for logic formulas is reasonable because before 12666, from 12666's sample file, a cell with formula "=2>1" will return 1.0. It is very strange. It should return "TRUE" like MS Excel.
So my fix will based on 12666.
The root cause is for logic formula cells, fix code of 12666 forcibly set output string to true or false. But many AOO users had manner already that set number format to logic formula cells. That's different with Excel. In Excel, no matter what number format you set to logic formula cells, the output strings are always true and false. In the sample of this issue, users set number format to currency, because of the forcibly setting, the string changed.
My solution is to add checking for logic formula cells, if their output number formats are certain category like number, currency, date, time, etc, we will not change the output string. but if no certain format categories were applied, for logic formula cells, will change output string to true and false.
Comment 16 Clarence GUO 2013-09-09 01:39:37 UTC
Just correct my pen mistake, all statement of 12666 in my previous comment should be 121126
Comment 17 Oliver-Rainer Wittmann 2013-09-09 14:04:25 UTC
My view on this issue:
I also figured out that the changes for bug 121126 triggered this issue.
The changes for 121126 has two parts.
One in the Microsoft Excel binary file format import filter code to not lose format codes on certain cells.
The second one is in the cell formatting code and changes AOO formatting behavior regarding boolean values. In Microsoft Excel the user is not able to format a boolean value as a number, date, currency, ..., as far as I have seen. Microsoft Excel treats boolean value as a number in formulas and calculation, but it looks like as it treats it as string/text in the formatting. In AOO until version 3.4.1 can format a boolean value a number or a currency and the corresponding format code is apply to the number interpretation of the boolean value. This AOO formatting behavior has been changed be the fix for bug 121162.

Thus, is this formatting change an improvement or a regression?
For whose which need to import Microsoft Excel files it is an improvement.
For whose who are making use of the AOO boolean formatting capability - like the reporter of this issue - it is a regression.

Now, I will have a closer look at Clarence's proposed solution to this dilemma.
Comment 18 Oliver-Rainer Wittmann 2013-09-10 12:59:30 UTC
I had a look at the proposed solution.

The patch tries to restrict the changed formatting behavior (introduced by bug 121126) to cells whose set formatting code category (<SvNumberformat::GetType()>) is not one of the number, date, currency one. Return value of <SvNumberformat::GetType()> is checked.

While playing around having the proposed patch applied I figured out the following.
- The definition NUMBERFORMAT_DEFINED (= 0x0001) has two purposes:
(1) Used for user-defined formatting code which could not be categories.
(2) Used as a flag for the other categories to indicate that it is a user-defined one. E.g., for a user-defined currency formatting code the return value of <SvNumberformat::GetType()> is 9 - "0x0008 | 0x0001" (NUMBERFORMAT_CURRENCY is defined as 0x0008

Thus, I decided to adjust the proposed conditions accordingly:
...
const short nFmtType = (pNumFmt->GetType() & ~NUMBERFORMAT_DEFINED);
if ( nFmtType == NUMBERFORMAT_DATE
     || nFmtType == NUMBERFORMAT_TIME
     || nFmtType == NUMBERFORMAT_CURRENCY
     || nFmtType == NUMBERFORMAT_NUMBER
     || nFmtType == NUMBERFORMAT_SCIENTIFIC
     || nFmtType == NUMBERFORMAT_FRACTION
     || nFmtType == NUMBERFORMAT_PERCENT
     || nFmtType == NUMBERFORMAT_DATETIME
     || pNumFmt->GetType() == NUMBERFORMAT_DEFINED )
...

But afterwards, Microsoft Excel document attached to bug 121126 shows wrong display for cells A5:A7. These cells have a user-defined number format - NUMBERFORMAT_NUMBER | NUMBERFORMAT_DEFINED - set.

The idea of Clarence' solution makes sense, but does not provide the expected result.

I think we need to search for another resp. improved solution.
Comment 19 Oliver-Rainer Wittmann 2013-09-10 14:01:06 UTC
May be it make sense to consider the given formatting code in order to decide, if the boolean result of a formula should be formatted as a number, currency, ... or as TRUE/FALSE.

The formatting codes of cells A5:A7 in Microsoft Excel document attached to bug 121126 contain a section for strings/text. The complete formatting code is:
    [BLUE]#,##0.00;[RED]\(#,##0.00\);[YELLOW]0;[GREEN]@
The section for strings/text is
    [GREEN]@

If formatting code for strings/text is available format the boolean result as TRUE/FALSE. Otherwise, apply the formatting as before.
Comment 20 Oliver-Rainer Wittmann 2013-09-10 15:05:36 UTC
Created attachment 81510 [details]
proposed solution based on comment 19
Comment 21 Regina Henschel 2013-09-10 20:06:22 UTC
Created attachment 81515 [details]
constants and TRUE() and FALSE() with different ways for formatting

Hi Oliver, I have applied your patch and build it. I have used the attached file to test it.

The ODF part looks good for me:
A forced currency format is applied to logical values for constants and for formula result.
A format code with text rule shows logical values as string.
The ISLOGICAL() test give correct results.

The Excel import has this error:
A logical constant is imported wrongly. The cell content is =TRUE() instead of constant TRUE and =FALSE() instead of constant FALSE. That is not an error. But despite the formulas =TRUE() and =FALSE(), which should result a logical value, the test ISLOGICAL() results FALSE.
To reproduce the Excel import error, save the attached file in *.xls format and reopen it. The error remains, if the *.xls file is opened in Excel 2010 and resaved there.
AOO3.4.1 imports logical constants as formulas too, but in AOO3.4.1 the type check with ISLOGICAL() passes.
Comment 22 Oliver-Rainer Wittmann 2013-09-11 08:00:20 UTC
(In reply to Regina Henschel from comment #21)
> Created attachment 81515 [details]
> constants and TRUE() and FALSE() with different ways for formatting
> 
> Hi Oliver, I have applied your patch and build it. I have used the attached
> file to test it.
> 

Thanks for the testing and the feedback.

> The ODF part looks good for me:
> A forced currency format is applied to logical values for constants and for
> formula result.
> A format code with text rule shows logical values as string.
> The ISLOGICAL() test give correct results.
> 
> The Excel import has this error:
> A logical constant is imported wrongly. The cell content is =TRUE() instead
> of constant TRUE and =FALSE() instead of constant FALSE. That is not an
> error. But despite the formulas =TRUE() and =FALSE(), which should result a
> logical value, the test ISLOGICAL() results FALSE.
> To reproduce the Excel import error, save the attached file in *.xls format
> and reopen it. The error remains, if the *.xls file is opened in Excel 2010
> and resaved there.

This import behavior has not been changed in AOO 4.0.0. It also occurs in OOo 3.3.0 and AOO 3.4.1

> AOO3.4.1 imports logical constants as formulas too, but in AOO3.4.1 the type
> check with ISLOGICAL() passes.

This seems to be also triggered by the changes made for bug 121126.
@Regina:
Please submit a separate issue for it. Thanks in advance.
Comment 23 Oliver-Rainer Wittmann 2013-09-11 08:20:37 UTC
(In reply to Oliver-Rainer Wittmann from comment #22)
> (In reply to Regina Henschel from comment #21)
> 
> [snip]
> 
> > AOO3.4.1 imports logical constants as formulas too, but in AOO3.4.1 the type
> > check with ISLOGICAL() passes.
> 
> This seems to be also triggered by the changes made for bug 121126.
> @Regina:
> Please submit a separate issue for it. Thanks in advance.

I checked some further scenarios, also in former AOO versions and also with documents created by Microsoft Excel. There are more issue than the above described one. Thus, further and deeper investigations are needed which should be done on a separate issue.
Comment 24 Clarence GUO 2013-09-11 08:36:28 UTC
I agree with Oliver's modification against my patch.
About the problem mentioned by Oliver with his new code, Microsoft Excel document attached to bug 121126 shows wrong display for cells A5:A7, I think it makes sense because MS Excel applied customized number format to these cells. If the manner(applying number format should work no matter what type of a formula returned) for old AOO users is a rule. According to the rule, the behavior is correct unless we want to change behavior 100% same with that in MS Excel.
Thus it's pity that the fix code of additional handling with logical formulas in 121126 is more or less redundant now because most number format can be categorized into number, currency, percent, date, time... formats with or without NUMBERFORMAT_DEFINED flag. So that code seldom work I think. But after all anyway it will work under some scenarios, for example, create a XLS by Excel and input "=2>1" to a cell, and apply text number format to it, in AOO3.4.0, it shows 1, but in AOO4.0 plus Oliver's new code, it shows TRUE like that in Excel. If you haven't apply text number format, in AOO3.4.0, there is no problem, also shows TRUE. I'm not sure whether returning 1 for a text logical format is a defect or not. It doesn't make sense from my personal perspective.
Comment 25 Clarence GUO 2013-09-11 08:47:12 UTC
Number format for cells A5:A7 in Microsoft Excel document attached to bug 121126 is "Custom", the format code is "[Blue]#,##0.00;[Red](#,##0.00);[Yellow]0;[Green]@". From the format code "0.00" we know it is a "Number" customized number format, so we get NUMBERFORMAT_NUMBER | NUMBERFORMAT_DEFINED for these cells is correct.
Comment 26 Oliver-Rainer Wittmann 2013-09-11 09:10:41 UTC
(In reply to Clarence GUO from comment #24)
> I agree with Oliver's modification against my patch.
> About the problem mentioned by Oliver with his new code, Microsoft Excel
> document attached to bug 121126 shows wrong display for cells A5:A7, I think
> it makes sense because MS Excel applied customized number format to these
> cells. If the manner(applying number format should work no matter what type
> of a formula returned) for old AOO users is a rule. According to the rule,
> the behavior is correct unless we want to change behavior 100% same with
> that in MS Excel.
> Thus it's pity that the fix code of additional handling with logical
> formulas in 121126 is more or less redundant now because most number format
> can be categorized into number, currency, percent, date, time... formats
> with or without NUMBERFORMAT_DEFINED flag. So that code seldom work I think.
> But after all anyway it will work under some scenarios, for example, create
> a XLS by Excel and input "=2>1" to a cell, and apply text number format to
> it, in AOO3.4.0, it shows 1, but in AOO4.0 plus Oliver's new code, it shows
> TRUE like that in Excel. If you haven't apply text number format, in
> AOO3.4.0, there is no problem, also shows TRUE. I'm not sure whether
> returning 1 for a text logical format is a defect or not. It doesn't make
> sense from my personal perspective.

Ok.
Thus, do you agree that my proposed solution could be integrated into branch AOO401 for our planned AOO 4.0.1 release?
Comment 27 Oliver-Rainer Wittmann 2013-09-11 09:12:15 UTC
(In reply to Clarence GUO from comment #25)
> Number format for cells A5:A7 in Microsoft Excel document attached to bug
> 121126 is "Custom", the format code is
> "[Blue]#,##0.00;[Red](#,##0.00);[Yellow]0;[Green]@". From the format code
> "0.00" we know it is a "Number" customized number format, so we get
> NUMBERFORMAT_NUMBER | NUMBERFORMAT_DEFINED for these cells is correct.

I agree that the category of this formatting code is correct.
I had no intention to change it.
Comment 28 Clarence GUO 2013-09-11 09:15:18 UTC
I agree with Oliver's new patch, it should be delivered into 4.0.1 branch
Comment 29 Oliver-Rainer Wittmann 2013-09-11 09:28:03 UTC
(In reply to Clarence GUO from comment #28)
> I agree with Oliver's new patch, it should be delivered into 4.0.1 branch

Thus, I am requesting release blocker flag
Comment 30 jsc 2013-09-11 11:03:43 UTC
approve showstopper request
Comment 31 SVN Robot 2013-09-11 15:01:16 UTC
"orw" committed SVN revision 1521885 into trunk:
122927: adjusting change made for 121126 - allow formatting of boolean formul...
Comment 32 SVN Robot 2013-09-11 15:10:21 UTC
"orw" committed SVN revision 1521894 into branches/AOO401:
122927: adjusting change made for 121126 - allow formatting of boolean formul...
Comment 33 Oliver-Rainer Wittmann 2013-09-11 15:14:46 UTC
fixed in trunk and branch AOO401
Comment 34 fanyuzhen 2013-09-17 08:46:33 UTC
Check with build AOO401m4(Build:9713)  -  Rev. 1521921, this formula now produced a result of $0.00 with " sample spreadsheet by email".