Bug 99093 - Wrong Data Validation in Calc
Summary: Wrong Data Validation in Calc
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
5.0.5.2 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.2.0
Keywords: bibisected, filter:xls, regression
Depends on:
Blocks:
 
Reported: 2016-04-05 05:14 UTC by Igor Yankovskiy
Modified: 2016-08-26 20:13 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Example workbook (23.00 KB, application/x-ole-storage)
2016-04-05 05:14 UTC, Igor Yankovskiy
Details
data validation that references a cell on the same row, to the left (23.00 KB, application/x-ole-storage)
2016-04-05 14:04 UTC, Igor Yankovskiy
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Igor Yankovskiy 2016-04-05 05:14:15 UTC
Created attachment 124079 [details]
Example workbook

1. Create new workbook in MS Excel 2010.

2. Set C5 cell value = "55" (for example)

3. In C6 cell set data validation: Whole number, less than C5.

4. Save file as Excel 97-2003 Workbook.

Then, if open this file in Calc, see Data Validation cell address Sheet1.C65541 (65541 = 65536 + 5)
Comment 1 raal 2016-04-05 12:23:04 UTC

 1e60fd0885f68be103b43036668ce3bb6061926f is the first bad commit
commit 1e60fd0885f68be103b43036668ce3bb6061926f
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Tue May 26 02:12:16 2015 -0500

    source 7be068e04fa36f238c26fd6747d66f0a1e01de00

    source 7be068e04fa36f238c26fd6747d66f0a1e01de00
    source 0a5c1be22a88094018e2741b38d743800018ca5e
    source 077043eb2e41471897fc5d415fa2f6a50b91ed92
    source ddc1f7d9a816e2cc970d48d2ccc2c0cd256e6e03
    source bb0c05cebed131805d04919fac2b83030087451b
    source a099abe9ffd5e90b7cb4d488e25df4fce3099a48
    source eeb3467569ef51640eb948f97668fa62597697b1
    source 8332bc98fac35ada2337de032bebfdae82139882
    source 7ba4405fca7ce0fa8f0d807375a162a344428294
    source e5f93bc97e39e7ff33dcf573186c8c6b4a4d587b
    source 21087baf1c9dae5e7e970f22d351d771210175b0
    source 6a2f0cecd249f672e74c3b08653ff56891c24071
    source 9364aee03b195422367026979940135429b40ffa
    source 2e293978d80c5113ea37f4c7f4b18ee01f2022e1
    source 422fdeccd88a89461271bd6d87774a4c5015ba60
    source e8c2bb2e7cbb476b4b73758fb9a053b5380b29f6
    source 36f1dec26699ed44dfc99858a201f8eac42dcf97
    source b387a3fe97478961c494ca087475cd968d79408b
    source b03a9ae9588f2d2ffd1460f6696d716f0799b769
    source 23a1717881ebfa3638b969aa4bad38a81d26d29d
    source 15174177091367332b57cd79575e2f7dd27388b2
    source 180e1de3f6bad36b00dfe3aeba43172e5e9a735e
    source 666fb8b7bc210be6d785515bc7660e5a5d19b82e
    source 806ced87cfe3da72df0d8e4faf5b82535fc7d1b7
    source d22519f62bcd1325f1e7cc920a115b68fccd1922
    source d44168795aed842d524e3a349962f2b98a8ac504
    source 33d00afddc66252fe9961d5396545e5f5a660700
    source 451cb9aace6d0d207b99691be025bd4bb8968a32
    source 1c2405ba44c5a146188c19e235f857ab18ea05f0
    source e96a5d4064a6002eb95b2c05f4e68c79bb766b07
    source 124a3e680fecf0659dfaf283a648a405c070c71c
    source ace38b3be1e1de01b8e484a768d5ce961e5eb689
    source 5f3e0690c01788b0b5867820342a52892766a526
    source 97c9bc9fada9cdfff956101a4a5e264b4dba58be
    source 924b2923b8b1515a4c37079e72a2a9cc6010a4f4


Tor, can it be this commit or some SC_ROW commits?  Thanks
author	Tor Lillqvist <tml@collabora.com>	2015-03-09 22:48:28 (GMT)
committer	Tor Lillqvist <tml@collabora.com>	2015-03-10 07:27:58 (GMT)
commit bb0c05cebed131805d04919fac2b83030087451b (patch)
tree 5ba10aaa12763a52f07cd8686894ac674f25075f
parent a099abe9ffd5e90b7cb4d488e25df4fce3099a48 (diff)
Also relative row references need to wrap around, like fdo#84556 for columns
Comment 2 How can I remove my account? 2016-04-05 13:22:35 UTC
That commit indeed looks like it could be guilty. Pity I not write anything more in the commit message.
Comment 3 How can I remove my account? 2016-04-05 13:29:59 UTC
I wonder if there is a similar issue for data validation that references a cell on the same row, to the left? Igor, could you please try in Excel 2010 to put in cell D5 the same data validation referring to C5, and again save as Excel 97-2003 Workbook, and check what Calc says about the D5 validation.
Comment 4 Igor Yankovskiy 2016-04-05 14:04:38 UTC
Created attachment 124088 [details]
data validation that references a cell on the same row, to the left

(In reply to Tor Lillqvist from comment #3)
> I wonder if there is a similar issue for data validation that references a
> cell on the same row, to the left? Igor, could you please try in Excel 2010
> to put in cell D5 the same data validation referring to C5, and again save
> as Excel 97-2003 Workbook, and check what Calc says about the D5 validation.

OK.
C5 value = "55"
D5 validation - Whole number, less than C5.
Save as .xls

In LibreOffice get Sheet1.IY5 (Sheet1!RC[255]), not Sheet1.C5.
Example in attachment (Refer left.xls).
Comment 5 Igor Yankovskiy 2016-04-05 14:11:50 UTC
Additionally, if files from MS Excel saved as .xlsx - everything works without error, references is OK.
Comment 6 How can I remove my account? 2016-04-05 14:19:49 UTC
Thanks Igor, that means that also moggi's fix for the column wrap-around case causes a similar problem then. Whew, I am not alone;)
Comment 7 raal 2016-04-05 14:24:16 UTC
(In reply to Igor Yankovskiy from comment #4)
> Created attachment 124088 [details]
> data validation that references a cell on the same row, to the left
> 
> (In reply to Tor Lillqvist from comment #3)
> > I wonder if there is a similar issue for data validation that references a
> > cell on the same row, to the left? Igor, could you please try in Excel 2010
> > to put in cell D5 the same data validation referring to C5, and again save
> > as Excel 97-2003 Workbook, and check what Calc says about the D5 validation.
> 
> OK.
> C5 value = "55"
> D5 validation - Whole number, less than C5.
> Save as .xls
> 
> In LibreOffice get Sheet1.IY5 (Sheet1!RC[255]), not Sheet1.C5.
> Example in attachment (Refer left.xls).

I see D5 = Sheet1.IY5.  Igor, see menu Tools> Options> Calc > Formula > formula syntax (mine is "calc A1)
Comment 8 raal 2016-04-05 14:25:14 UTC
(In reply to raal from comment #7)
> (In reply to Igor Yankovskiy from comment #4)
> > Created attachment 124088 [details]
> > data validation that references a cell on the same row, to the left
> > 
> > (In reply to Tor Lillqvist from comment #3)
> > > I wonder if there is a similar issue for data validation that references a
> > > cell on the same row, to the left? Igor, could you please try in Excel 2010
> > > to put in cell D5 the same data validation referring to C5, and again save
> > > as Excel 97-2003 Workbook, and check what Calc says about the D5 validation.
> > 
> > OK.
> > C5 value = "55"
> > D5 validation - Whole number, less than C5.
> > Save as .xls
> > 
> > In LibreOffice get Sheet1.IY5 (Sheet1!RC[255]), not Sheet1.C5.
> > Example in attachment (Refer left.xls).
> 
> I see D5 = Sheet1.IY5.  Igor, see menu Tools> Options> Calc > Formula >
> formula syntax (mine is "calc A1)

I misread your comment..
Comment 9 Markus Mohrhard 2016-04-05 17:59:18 UTC
(In reply to Tor Lillqvist from comment #6)
> Thanks Igor, that means that also moggi's fix for the column wrap-around
> case causes a similar problem then. Whew, I am not alone;)

Interestingly mso-dumper reports what we expect:

01BEh: =====================================================================
01BEh: DV - Data Validation Criteria (01BEh)
01BEh:   size = 43
01BEh: ---------------------------------------------------------------------
01BEh: 01 01 5C 00 01 00 00 00 01 00 00 00 01 00 00 00   ..\.............
01BEh: 01 00 00 00 05 00 00 00 4C 00 00 FF C0 00 00 FF   ........L.......
01BEh: FF 01 00 04 00 04 00 03 00 03 00                  ...........
01BEh: ---------------------------------------------------------------------
01BEh: type: whole number (0x1)
01BEh: error style: stop icon (0x0)
01BEh: list of valid inputs: no
01BEh: allow blank: yes
01BEh: suppress down-down in cell: no
01BEh: IME mode: No Control (0x0)
01BEh: show input message: yes
01BEh: show error message: yes
01BEh: operator type: Less Than (0x5)
01BEh: formula 1 (bytes): 4C 00 00 FF C0
01BEh: formula 1 (displayed): (single-ref: col=255[rel],row=0[rel])
01BEh: formula 2 (bytes): 
01BEh: formula 2 (displayed): 
01BEh: range: $D$5:$D$5


So basically we still have a wrap around case but I think we missed the wrap around the other side case. In the code here we have a 4 + 255 = 259; if we now calculate this mod 256 we get the expected result of 3.

I'll fix both commits and a add a set of tests for these conditions.
Comment 10 Markus Mohrhard 2016-04-06 03:16:06 UTC
(In reply to Markus Mohrhard from comment #9)
> (In reply to Tor Lillqvist from comment #6)
> > Thanks Igor, that means that also moggi's fix for the column wrap-around
> > case causes a similar problem then. Whew, I am not alone;)
> 
> Interestingly mso-dumper reports what we expect:
> 
> 01BEh: =====================================================================
> 01BEh: DV - Data Validation Criteria (01BEh)
> 01BEh:   size = 43
> 01BEh: ---------------------------------------------------------------------
> 01BEh: 01 01 5C 00 01 00 00 00 01 00 00 00 01 00 00 00   ..\.............
> 01BEh: 01 00 00 00 05 00 00 00 4C 00 00 FF C0 00 00 FF   ........L.......
> 01BEh: FF 01 00 04 00 04 00 03 00 03 00                  ...........
> 01BEh: ---------------------------------------------------------------------
> 01BEh: type: whole number (0x1)
> 01BEh: error style: stop icon (0x0)
> 01BEh: list of valid inputs: no
> 01BEh: allow blank: yes
> 01BEh: suppress down-down in cell: no
> 01BEh: IME mode: No Control (0x0)
> 01BEh: show input message: yes
> 01BEh: show error message: yes
> 01BEh: operator type: Less Than (0x5)
> 01BEh: formula 1 (bytes): 4C 00 00 FF C0
> 01BEh: formula 1 (displayed): (single-ref: col=255[rel],row=0[rel])
> 01BEh: formula 2 (bytes): 
> 01BEh: formula 2 (displayed): 
> 01BEh: range: $D$5:$D$5
> 
> 
> So basically we still have a wrap around case but I think we missed the wrap
> around the other side case. In the code here we have a 4 + 255 = 259; if we
> now calculate this mod 256 we get the expected result of 3.
> 
> I'll fix both commits and a add a set of tests for these conditions.

Actually there is a second problem. The validation import does not set the position because as we can see in the mso-dumper output that the range is imported after the formula. I fear I need to think a bit about that.
Comment 11 Commit Notification 2016-04-07 12:08:42 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

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

we need the position in the formula converter, tdf#99093

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 12 Commit Notification 2016-04-07 19:14:48 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

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

add test for tdf#99093

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 13 Igor Yankovskiy 2016-08-26 20:13:47 UTC
After update to 5.2.0 release bug seems fixed. Thanks!