Bug 130104 - FILESAVE XLSX: cell indent ("stylesheet -> cellXfs -> xf -> alignment: indent") is increased each time when re-save as xlsx document (see comment 17)
Summary: FILESAVE XLSX: cell indent ("stylesheet -> cellXfs -> xf -> alignment: indent...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.2.0.4 release
Hardware: All All
: medium normal
Assignee: Kevin Suo
URL:
Whiteboard: target:7.3.0 target:7.2.3
Keywords: bibisected, bisected, implementationError
: 67774 112220 (view as bug list)
Depends on:
Blocks: XLSX
  Show dependency treegraph
 
Reported: 2020-01-21 09:24 UTC by jdobry
Modified: 2022-09-14 15:23 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample document before open in Calc (7.67 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2020-01-21 09:26 UTC, jdobry
Details
Step 1 screenshoot (23.87 KB, image/png)
2020-01-21 09:27 UTC, jdobry
Details
Step 3 screenshoot (10.39 KB, image/png)
2020-01-21 09:28 UTC, jdobry
Details
test-indent-original.xlsx (5.33 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2021-06-26 06:27 UTC, Kevin Suo
Details
test-indent-firstSave.xlsx (5.32 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2021-06-26 06:28 UTC, Kevin Suo
Details
tdf130104_indent.xlsx (9.08 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2021-10-07 14:28 UTC, Kevin Suo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jdobry 2020-01-21 09:24:56 UTC
Description:
MSOffice is able to format cell to align right (or left) with indentation.
LibreOffice Calc broke this spreadsheet more and more on every save (it not need modification save is enough)

Steps to Reproduce:
1. Create XLSX document in MSoffice. Only one cell with formatting -> align to right, indent > 0 and save it. (Example document is in attachment, screenshot in attachment)
2. Open this document on LibreOffice calc, save it and close.
3. Repeat step 2 several times. Content of cell move to the left out of boundary. (screenshot in attachment)

Actual Results:
LibreOffice Calc broke this spreadsheet.

Expected Results:
LibreOffice Calc don't need support this feature. Not destroying spreadsheet is enough. 


Reproducible: Always


User Profile Reset: No



Additional Info:
Comment 1 jdobry 2020-01-21 09:26:26 UTC
Created attachment 157293 [details]
Sample document before open in Calc
Comment 2 jdobry 2020-01-21 09:27:38 UTC
Created attachment 157294 [details]
Step 1 screenshoot
Comment 3 jdobry 2020-01-21 09:28:16 UTC
Created attachment 157295 [details]
Step 3 screenshoot
Comment 4 Durgapriyanka 2020-01-21 15:56:07 UTC
Thank you for reporting the bug. I can confirm the bug present in

Version: 6.4.0.0.alpha1+ (x86)
Build ID: ec7374ff84c71edfbb30d6e4dc5b486b6df7107f
CPU threads: 2; OS: Windows 6.1 Service Pack 1 Build 7601; UI render: default; VCL: win; 
TinderBox: Win-x86@42, Branch:master, Time: 2019-11-10_21:37:30
Locale: en-US (en_US); UI-Language: en-US
Calc: threaded
	

But,not in

LibreOffice 3.3.0 
OOO330m19 (Build:6)
tag libreoffice-3.3.0.4
Comment 5 Xisco Faulí 2020-01-21 16:04:23 UTC
Reproduced in


Version: 4.3.0.0.alpha1+
Build ID: c15927f20d4727c3b8de68497b6949e72f9e6e9e

but not in

Version 4.1.0.0.alpha0+ (Build ID: efca6f15609322f62a35619619a6d5fe5c9bd5a)
Comment 6 jdobry 2020-01-21 16:28:32 UTC
I found this problem on version 6.1.4 originally. This behavior was my motivation for upgrade (to 6.3.4.2) and it doesn't help.
Comment 7 Aron Budea 2020-02-03 04:07:57 UTC
This isn't really a regression, as previously the indent wasn't applied properly, but the point of change could be bibisected using repo bibisect-42max.

https://cgit.freedesktop.org/libreoffice/core/commit/?id=af169a601dee381b17d1e87c346f2d97bad9a69f
author		abdulmajeed ahmed <aalabdulrazzaq@kacst.edu.sa>	2013-05-31 10:05:55 +0200
committer	abdulmajeed ahmed <aalabdulrazzaq@kacst.edu.sa>	2013-05-31 10:27:14 +0200

Fix fdo#51835 Indent changes in Calc forces alignment to left
Comment 8 jdobry 2020-02-25 08:39:56 UTC
It looks like duplicate to 

https://bugs.documentfoundation.org/show_bug.cgi?id=112220
Comment 9 Kevin Suo 2020-02-25 13:54:35 UTC
*** Bug 112220 has been marked as a duplicate of this bug. ***
Comment 10 Kevin Suo 2020-11-18 12:40:42 UTC
Set importance to HIGH MAJOR as this causes format loss.
Comment 11 Xisco Faulí 2020-11-18 12:50:37 UTC
format loss is different than data loss
Comment 12 reginabrown3000 2021-05-10 20:00:44 UTC
I also have this issue in 6.4.7.2 on an xlsx sheet that is saved as an xlsx sheet. I don't remember whether the original was created in Excel, but it probably was.
Comment 13 Kevin Suo 2021-06-26 06:25:19 UTC Comment hidden (obsolete)
Comment 14 Kevin Suo 2021-06-26 06:27:28 UTC
Created attachment 173214 [details]
test-indent-original.xlsx
Comment 15 Kevin Suo 2021-06-26 06:28:08 UTC
Created attachment 173215 [details]
test-indent-firstSave.xlsx
Comment 16 Kevin Suo 2021-06-26 06:46:34 UTC
Although I know the reason, it is out of my knowledge to fix this. I tried to change the code to:
> sal_Int32 nIndent = getUnitConverter().scaleToMm100( maModel.mnIndent, Unit::Space )

and after this change there is no increase of indent after each save anymore, but the indent width shown after reopen seems to be a little different than the original with as shown on screen.
Comment 17 Kevin Suo 2021-07-01 08:29:56 UTC
My bad...

It was not FILEOPEN, it is FILESAVE.

On XLSX import the indent is correctly detected (as integer value such as "1"), and is converted to a value which is 3 times the width of space in 100mm (such as 528), in
https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/stylesbuffer.cxx?r=9964531f#1186
> sal_Int32 nIndent = getUnitConverter().scaleToMm100( 3.0 * maModel.mnIndent, Unit::Space );

However, when you now save the document, the width value in 100mm unit is then converted to the following:
https://opengrok.libreoffice.org/xref/core/sc/source/filter/excel/xestyle.cxx?r=b8cfea65#1461
> nTmpIndent = (nTmpIndent + 100) / 200;
The comment in this line says "1 Excel unit == 10 pt == 200 twips", so this 100mm unit value is treated as pt and converted to twips. The problem is that in xlsx file the indent value should be an integer number such, of which "1" means 3 space width, as correctly used in sc/source/filter/oox/stylesbuffer.cxx:1186.

So the solution would be convert back the 100mm unit value to int value, which is a reverse operation of the import calculation. I see there is UnitConverter::scaleFromMm100 function in 
https://opengrok.libreoffice.org/s?refs=scaleFromMm100&project=core
which can do this.
Comment 18 Kevin Suo 2021-07-01 10:50:09 UTC Comment hidden (obsolete)
Comment 19 Buovjaga 2021-09-15 16:21:34 UTC
*** Bug 144116 has been marked as a duplicate of this bug. ***
Comment 20 Kevin Suo 2021-10-05 16:13:56 UTC
I have submitted a patch on gerrit:
https://gerrit.libreoffice.org/c/core/+/123111

Please review and provide feedback.
Comment 21 Kevin Suo 2021-10-06 13:04:50 UTC
Reset assignee to default as I have abandoned the patch on gerrit, see discussion in:
https://gerrit.libreoffice.org/c/core/+/123111
Comment 22 Kevin Suo 2021-10-07 14:28:55 UTC
Created attachment 175572 [details]
tdf130104_indent.xlsx
Comment 23 Commit Notification 2021-10-08 06:02:13 UTC
Kevin Suo committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6b93ee72df1aa42d1a3482ffc396bd0c23134f8b

tdf#130104 - FILESAVE XLSX: cell indent increased on each save

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 24 Kevin Suo 2021-10-08 06:09:26 UTC
Big thanks to Mike Kaganski who has helped me fix this. With the patch merged above now the indent upon roundtrip is reserved. 

However, as stated on the commit message, the UI part need to be improved, so that after xlsx import, if the user hit the "Increase Indent" or "Decrease Indent" toolbar icon to change the indent, Calc should be able to detect that we are operating in an xlsx file, thus the "increment" should be 3 * width of space char, rather than the current SC_INDENT_STEP. Also, the if the user changes the default font of the xlsx document, then Calc should recalculate the indent for each cell to reflect the possible change in width of space char. I will open a new bug for this issue.

In addition, on xlsx import of the file tdf130104_indent.xlsx, the default font (i.e. font for the "Normal" style) is "Times New Roman". However, when the UI locale is set to Simplified Chinese and "Asian" option is enabled in Tools -> Options -> Language Settigns -> Languages -> "Default Languages for Documents", upon resave as xlsx, the default font for the document is changed to "Noto Sans CJK SC" on my system, which causes the space-width detected on export to be different from the width detected on xlsx import. This seems to be bug 131349.
Comment 25 Commit Notification 2021-10-22 06:29:40 UTC
Kevin Suo committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/7982723a2943d8dcfc533a507880ab5991a04908

tdf#130104 - FILESAVE XLSX: cell indent increased on each save

It will be available in 7.2.3.

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 26 Kevin Suo 2022-09-14 14:39:20 UTC
*** Bug 67774 has been marked as a duplicate of this bug. ***