Bug 143896 - FILESAVE XLSX 1001+ rows are lost after saving new file
Summary: FILESAVE XLSX 1001+ rows are lost after saving new file
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.2.0.0 alpha1+
Hardware: All All
: highest critical
Assignee: Julien Nabet
URL:
Whiteboard: target:7.3.0 target:7.2.0
Keywords: bibisected, bisected, dataLoss, filter:xlsx, regression
Depends on:
Blocks: Print-Range
  Show dependency treegraph
 
Reported: 2021-08-16 09:10 UTC by NISZ LibreOffice Team
Modified: 2021-08-17 22:56 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample data as plain text (7.71 KB, text/plain)
2021-08-16 09:10 UTC, NISZ LibreOffice Team
Details
The sample data inserted to Calc (157.80 KB, image/png)
2021-08-16 09:12 UTC, NISZ LibreOffice Team
Details
The sample data saved by Calc (13.13 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2021-08-16 09:12 UTC, NISZ LibreOffice Team
Details
Screenshot of the problem in Calc after reloading (22.56 KB, image/png)
2021-08-16 09:13 UTC, NISZ LibreOffice Team
Details

Note You need to log in before you can comment on or make changes to this bug.
Description NISZ LibreOffice Team 2021-08-16 09:10:08 UTC
Created attachment 174308 [details]
Sample data as plain text

When a new file is created with more than 1001 rows and saved as XLSX the rows over 1001 are lost on save.

Steps to reproduce:
    1. Open the attached text file, it contains numbers 1-1500 in rows
    2. Paste all of this as unformatted text into a new Calc file
    3. Save as XLSX, reload in Calc or Excel

Actual results:
Row 1001 is the last one still having contents.

Expected results:
All rows retained.

LibreOffice details:
Version: 7.3.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 5aac78e5fb241050a86714687e9ff8804588ae3c
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: Skia/Raster; VCL: win
Locale: hu-HU (hu_HU); UI: hu-HU
Calc: CL

Additional Information:
Bibisected using bibisect-win64-7.2 to:
URL: https://cgit.freedesktop.org/libreoffice/core/commit/?id=2bf3e0d00e3bccb5b250642ee0d3fdbe6cae8ecc 

author	 Attila Szűcs <szucs.attila3@nisz.hu>	2021-01-27 17:43:43 +0100
committer	László Németh <nemeth@numbertext.org>	2021-02-09 16:37:51 +0100
commit 2bf3e0d00e3bccb5b250642ee0d3fdbe6cae8ecc (patch)

tdf#104502 sc: skip hidden columns at printing pages

Adding CC to: Attila Szűcs

Note: this might be related to the fix added to bug 41425 which introduced an 1000 row limit to save rows with formatting but without contents to xlsx. 
See also bug 46738 for lifting this limit and properly save any number of rows with formatting but without contents to xlsx.
Comment 1 NISZ LibreOffice Team 2021-08-16 09:12:31 UTC
Created attachment 174309 [details]
The sample data inserted to Calc
Comment 2 NISZ LibreOffice Team 2021-08-16 09:12:53 UTC
Created attachment 174310 [details]
The sample data saved by Calc
Comment 3 NISZ LibreOffice Team 2021-08-16 09:13:36 UTC
Created attachment 174311 [details]
Screenshot of the problem in Calc after reloading
Comment 4 Aron Budea 2021-08-16 10:17:06 UTC
Confirmed using LO 7.3.0.0.alpha0+ (c97bdca2d7abf30a99c45192b8f49c5bf76ca035) / Ubuntu.
Comment 5 Julien Nabet 2021-08-16 15:37:10 UTC
On pc Debian x86-64 with master sources updated today, I could reproduce this.
I don't understand why it happens only when saving in xlsx and not in ods for example.
Indeed, I don't see anything specific to xlsx in the quoted patch.
Comment 6 Xisco Faulí 2021-08-16 15:45:12 UTC
(In reply to Julien Nabet from comment #5)
> On pc Debian x86-64 with master sources updated today, I could reproduce
> this.
> I don't understand why it happens only when saving in xlsx and not in ods
> for example.
> Indeed, I don't see anything specific to xlsx in the quoted patch.

Also happening when exporting to XLS. This is a blocker for 7.2 release
Comment 7 Julien Nabet 2021-08-16 16:06:49 UTC
I gave a try here:
https://gerrit.libreoffice.org/c/core/+/120552
Comment 8 Xisco Faulí 2021-08-16 16:25:04 UTC
(In reply to Julien Nabet from comment #7)
> I gave a try here:
> https://gerrit.libreoffice.org/c/core/+/120552

Hi Julien,
Are you comfortable with the patch being submitting to master and then backported to 7.2 and 7.2.0, otherwise, we should go with the revert for now. Either way, the fix or the revert has to be in 7.2.0
Comment 9 Julien Nabet 2021-08-16 16:32:38 UTC
(In reply to Xisco Faulí from comment #8)
> (In reply to Julien Nabet from comment #7)
> > I gave a try here:
> > https://gerrit.libreoffice.org/c/core/+/120552
> 
> Hi Julien,
> Are you comfortable with the patch being submitting to master and then
> backported to 7.2 and 7.2.0, otherwise, we should go with the revert for
> now. Either way, the fix or the revert has to be in 7.2.0

I won't say "quite confortable" since I don't know how it works.
I just noticed this:
git grep -n bTableAreaVisibleValid
sc/inc/table.hxx:238:    mutable bool    bTableAreaVisibleValid:1;
sc/source/core/data/table1.cxx:270:    bTableAreaVisibleValid(false),
sc/source/core/data/table1.cxx:564:        if (!bTableAreaVisibleValid)
sc/source/core/data/table1.cxx:567:            bTableAreaVisibleValid = true;

git grep -n bTableAreaValid
sc/inc/table.hxx:237:    mutable bool    bTableAreaValid:1;
sc/source/core/data/table1.cxx:269:    bTableAreaValid(false),
sc/source/core/data/table1.cxx:554:        if (!bTableAreaValid)
sc/source/core/data/table1.cxx:557:            bTableAreaValid = true;
sc/source/core/data/table2.cxx:1527:    bTableAreaValid = false;

So thought there could be 1 use of "bTableAreaVisibleValid" lacking. Moreover, this var is new from this patch.
I can just tell I don't reproduce the pb with this patch.
Now we can also wait for Jenkins results and its check tests.
Comment 10 Xisco Faulí 2021-08-16 16:35:44 UTC
(In reply to Julien Nabet from comment #9)
> (In reply to Xisco Faulí from comment #8)
> > (In reply to Julien Nabet from comment #7)
> > > I gave a try here:
> > > https://gerrit.libreoffice.org/c/core/+/120552
> > 
> > Hi Julien,
> > Are you comfortable with the patch being submitting to master and then
> > backported to 7.2 and 7.2.0, otherwise, we should go with the revert for
> > now. Either way, the fix or the revert has to be in 7.2.0
> 
> I won't say "quite confortable" since I don't know how it works.
> I just noticed this:
> git grep -n bTableAreaVisibleValid
> sc/inc/table.hxx:238:    mutable bool    bTableAreaVisibleValid:1;
> sc/source/core/data/table1.cxx:270:    bTableAreaVisibleValid(false),
> sc/source/core/data/table1.cxx:564:        if (!bTableAreaVisibleValid)
> sc/source/core/data/table1.cxx:567:            bTableAreaVisibleValid = true;
> 
> git grep -n bTableAreaValid
> sc/inc/table.hxx:237:    mutable bool    bTableAreaValid:1;
> sc/source/core/data/table1.cxx:269:    bTableAreaValid(false),
> sc/source/core/data/table1.cxx:554:        if (!bTableAreaValid)
> sc/source/core/data/table1.cxx:557:            bTableAreaValid = true;
> sc/source/core/data/table2.cxx:1527:    bTableAreaValid = false;
> 
> So thought there could be 1 use of "bTableAreaVisibleValid" lacking.
> Moreover, this var is new from this patch.
> I can just tell I don't reproduce the pb with this patch.
> Now we can also wait for Jenkins results and its check tests.

yep, let's wait. Another option would be to revert it in 7.2/7.2.0 only and apply your patch to master only. it looks much safer that way
Comment 11 Julien Nabet 2021-08-16 16:40:43 UTC
(In reply to Xisco Faulí from comment #10)
> ...
> 
> yep, let's wait. Another option would be to revert it in 7.2/7.2.0 only and
> apply your patch to master only. it looks much safer that way

About reverting, I can just say there'll be a conflict to solve on master or on libreoffice-7-2 branch.
Comment 12 Xisco Faulí 2021-08-16 16:45:48 UTC
(In reply to Julien Nabet from comment #11)
> (In reply to Xisco Faulí from comment #10)
> > ...
> > 
> > yep, let's wait. Another option would be to revert it in 7.2/7.2.0 only and
> > apply your patch to master only. it looks much safer that way
> 
> About reverting, I can just say there'll be a conflict to solve on master or
> on libreoffice-7-2 branch.

Yep, they are already fixed in the latest patch of https://gerrit.libreoffice.org/c/core/+/120444
Comment 13 Commit Notification 2021-08-16 18:49:19 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/faee2796e3f3d98521b4b23b167b688171560254

tdf#143896: FILESAVE XLS(X) 1001+ rows are lost after saving new file

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Julien Nabet 2021-08-16 18:56:18 UTC
Thank you Xisco for the review.
I saw the patch for 7.2 branch here:https://gerrit.libreoffice.org/c/core/+/120449

I didn't find the one for 7.2.0 branch, do you still want we cherry-pick it for this branch too?
Comment 15 Xisco Faulí 2021-08-16 19:16:09 UTC
(In reply to Julien Nabet from comment #14)
> Thank you Xisco for the review.
> I saw the patch for 7.2 branch
> here:https://gerrit.libreoffice.org/c/core/+/120449
> 
> I didn't find the one for 7.2.0 branch, do you still want we cherry-pick it
> for this branch too?

yep, but make sure jenkins passes in 7.2 branch first
Comment 16 Commit Notification 2021-08-16 19:57:33 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/ab6c339faa15bf324f1161678717f02dbf8c19f6

tdf#143896: FILESAVE XLS(X) 1001+ rows are lost after saving new file

It will be available in 7.2.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2021-08-16 20:05:47 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ea7b9f87556414d37b83a5dd45cd944d97597ad1

tdf#143896: sc_uicalc: Add unittest

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2021-08-16 21:39:39 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/1a73d70ffc834ff4ff29e13b1d1c06be60ce58b5

tdf#143896: sc_uicalc: Add unittest

It will be available in 7.2.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 Xisco Faulí 2021-08-17 09:48:20 UTC
Verified in

Version: 7.3.0.0.alpha0+ / LibreOffice Community
Build ID: 615a340fcf05845397ea3c9917e2eadf074b4514
CPU threads: 4; OS: Linux 5.10; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

@Julien, thanks for fixing this issue so fast!!
Comment 20 Commit Notification 2021-08-17 22:51:28 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-2-0":

https://git.libreoffice.org/core/commit/376d9889bcd61dbfc0531b2168e2f04736d81d49

tdf#143896: FILESAVE XLS(X) 1001+ rows are lost after saving new file

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.