Bug 32950 - FILEOPEN XLS: Cell height may be wrong because 'Optimal height" of rows is not re-calculated at load time.
Summary: FILEOPEN XLS: Cell height may be wrong because 'Optimal height" of rows is no...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: low minor
Assignee: Not Assigned
URL:
Whiteboard: interoperability
Keywords: filter:xls, needsDevEval
: 39486 91234 (view as bug list)
Depends on:
Blocks: Calc-Cells XLS
  Show dependency treegraph
 
Reported: 2011-01-09 23:03 UTC by Michael Kazarian
Modified: 2024-03-12 18:19 UTC (History)
11 users (show)

See Also:
Crash report or crash signature:


Attachments
Speadsheet with bugs demo (719.80 KB, application/x-gzip)
2011-01-09 23:03 UTC, Michael Kazarian
Details
wrong word wrap in print preview mode vs edit mode (231.85 KB, application/gzip)
2013-06-04 09:16 UTC, Kevin Suo
Details
XLS sample A (7.50 KB, application/x-ole-storage)
2015-10-17 08:19 UTC, gmarco
Details
XLS sample B (7.00 KB, application/x-ole-storage)
2015-10-17 08:25 UTC, gmarco
Details
XLS sample C (12.00 KB, application/vnd.ms-excel)
2022-06-06 07:26 UTC, Vladimir M.
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Kazarian 2011-01-09 23:03:22 UTC
Created attachment 41824 [details]
Speadsheet with bugs demo

When I open some (not all) xls-files in LibreOffice I have got wrong formatting height of row (see row_styles.xls and row_styles2.xls) and wrap words (see row_styles1.xls). 

These bugs is reproductible only in LibreOffice on Linux and Windows (I tested only on x86 architecture). Sun-Oracle versions haven't this bugs (see attachment pictures for detail). 
LibreOffice-row_styles-bug.png - show opened files with bugs in LibreOffice 
OracleOpenOfficeorg-row_styles.png and SunOpenOfficeorg-row_styles.png - show opened files without bugs in Sun/Oracle versions of OpenOffice.org

Regards 
Michael Kazarian.
Comment 1 Kohei Yoshida 2011-01-10 20:51:15 UTC Comment hidden (obsolete)
Comment 2 Aurimas Fišeras 2011-03-29 02:58:13 UTC Comment hidden (obsolete)
Comment 3 Michael Kazarian 2011-03-29 22:21:52 UTC
(In reply to comment #2)
> I think bug https://bugs.freedesktop.org/show_bug.cgi?id=34717 is related to
> this one.

No. This bug appear from Go-oo 3.1 and present in higher versions. It's Go-oo problem, not in original OpenOffice.org
Comment 4 Michael Kazarian 2012-02-14 23:44:25 UTC Comment hidden (obsolete)
Comment 5 Kohei Yoshida 2012-02-15 07:50:46 UTC
Sorry. Got busy. I might turn this into an EasyHack so that others (which could be you) can work on it.

The tricky part is that OOo's solution was a sledge hammer approach that massively hurt performance for *all* documents (they simply re-calculated row heights on all rows, which was super slow).  So our solution must be sensible performance-wise, by identifying the rows that need re-calculating and only re-calc heights on those rows.

BTW strictly speaking we are faithfully importing row heights per what the Excel document specifies.  Excel seems to amend row heights on some of them on import when those specified row heights are not high enough (for whatever reason).  The right fix is to search for any clues in the file format as to which rows need re-calculation.

Setting status to NEW as I'm not working on this.  If anyone wants to look into it let me know. I'll provide a code pointer.
Comment 6 Florian Reisinger 2012-03-25 06:29:16 UTC
Not working @ Ubuntu 11.10 LibreOfficce 3.4.4 OOO340m1 (Build:402)
I will have a look on 3.4 soon....
Comment 7 Michael Kazarian 2012-03-25 22:08:10 UTC
(In reply to comment #6)
> Not working @ Ubuntu 11.10 LibreOfficce 3.4.4 OOO340m1 (Build:402)

Florian, this bug in LO 3.5 too. See comment 5 from Kohei Yoshida for details.
Comment 8 Kohei Yoshida 2012-05-08 13:28:00 UTC
Reference to the gnumeric bugzilla.

https://bugzilla.gnome.org/show_bug.cgi?id=614399

They are having the same issue there as well.
Comment 9 Kevin Suo 2013-06-04 09:16:05 UTC
Created attachment 80274 [details]
wrong word wrap in print preview mode vs edit mode

I think the situation in my attachment is related to this bug.

In print preview mode, it show different word wrap vs edit mode. 

I'm in Ubuntu 13.04 with LibreOffice 4.0.3.3.
Comment 10 Michael Kazarian 2013-06-05 05:51:07 UTC
> I think the situation in my attachment is related to this bug.
> In print preview mode, it show different word wrap vs edit mode. 
Unlikely, I think. In my situation preview and edit mode behave equally. You have found other bug.
Comment 11 Kevin Suo 2013-06-05 06:45:17 UTC
> Unlikely, I think. In my situation preview and edit mode behave
> equally. You have found other bug.

This bug shows when set the column width almost the same as the text in
the auto-wrap cell, while text in that cell is in a single line.
Maybe I should file another bug report.
Comment 12 Michael Kazarian 2013-06-05 11:47:36 UTC
> Maybe I should file another bug report.
I think, you would report about your bug. LO developers will understand about related whether our bugs.
Comment 13 Bernhard Dippold 2013-10-30 09:03:18 UTC
Kohei's description in comment 5 might be an explanation for bug 62268 and bug 62361 too.
If the performance issue caused ignorance of "style:use-optimal-row-height='true'" in LibO since version 3.5.7.2, we know at least a reason - the next step would be a solution...
(I don't add 62268 and 62361 to the related bugs list, as this still might be another reason)
Comment 14 Joel Madero 2014-12-15 01:48:03 UTC
Verified: 
Ubuntu 14.04 x64
Version: 4.5.0.0.alpha0+
Build ID: 8b65be4740f4349b769a8709867e0cc32d93686d

Changing priority according to priority flowchart: https://wiki.documentfoundation.org/images/0/06/Prioritizing_Bugs_Flowchart.jpg

Minor - can slow down professional quality work but will not prevent it.
Low - default seems appropriate.

Also:
Verified on:
Ubuntu 14.04 x64
LibreOffice 3.3 (inherited from OOo)

Updating version as version is the oldest version that the bug has been confirmed on.


Thanks for your patience and understanding in this issue. LibreOffice is powered by volunteers who donate thousands of hours for free to help improve LibreOffice for everyone.
Comment 15 gmarco 2015-10-17 08:19:53 UTC
Created attachment 119684 [details]
XLS sample A
Comment 16 gmarco 2015-10-17 08:25:43 UTC
Created attachment 119685 [details]
XLS sample B

I have encountered a similar case in LOC 4.4.5.2 - Windows 8.1, I thought it was a bug, but after some tests and checks have established the following.
The original sheet is an XLS (Excel XP) in which the rows are formatted with "fixed" height (not "optimal") because the width of the original column was larger and did not have the need.
Migrating to LO and opening it in the Calc spreadsheet, it retains those settings correctly.
Having now the need to restrict the column and consequently to set the "Wrap text automatically", this feature does not apply if you do not change the formatting of the row by setting "Optimal Height" (what would happen in Excel too).
As evidence of the above I attach two files XLS: "SampleTest A" has lines formatted with "fixed" height and opening it with Calc gets the problem; "SampleTest B" has the line 5 formatted "optimal" and gets no problem.
Comment 17 Cor Nouws 2015-10-30 07:33:26 UTC
changing summary accoring to findings in bug 95098
Comment 18 Robinson Tryon (qubit) 2015-11-19 00:37:42 UTC Comment hidden (obsolete)
Comment 19 Robinson Tryon (qubit) 2015-12-14 06:13:31 UTC Comment hidden (obsolete)
Comment 20 Joel Madero 2016-07-04 07:16:23 UTC Comment hidden (obsolete)
Comment 21 m_a_riosv 2017-05-02 01:00:32 UTC Comment hidden (obsolete)
Comment 22 m_a_riosv 2017-05-02 01:01:57 UTC
*** Bug 91234 has been marked as a duplicate of this bug. ***
Comment 23 QA Administrators 2018-06-01 02:16:35 UTC Comment hidden (obsolete)
Comment 24 mirh 2020-05-23 16:12:02 UTC
Still a thing after 9 years.
Comment 25 QA Administrators 2022-05-24 03:33:20 UTC Comment hidden (obsolete)
Comment 26 Vladimir M. 2022-06-06 07:26:34 UTC
Created attachment 180585 [details]
XLS sample C

I don't know if what I encountered is related to this bug but here's what I've found.
I have an xls-file that was generated by a system that we work with. I don't know any details on how it was created.
When I opened this file in LibreOffice (tried 7.2.7, 7.3.3 and 3.3.4 as previous comment suggested) the height of rows was incorrect.
When I opened it in Microsoft Excel 2016 the height was correct. After I saved it in Excel (without changing anything) and reopened it in LibreOffice the height was also correct.
The file has a lot of sheets so I'll attach just one.

I also resaved row_styles1.xls and row_styles2.xls in Excel 2016 from the original post. The result was the same: now they both have correct row height in LibreOffice.

Maybe this information will help.
Comment 27 Justin L 2022-09-14 23:42:26 UTC
repro 7.5+

Based on comment 5, I'd guess this is the XLS version bug 62268 and needs something like the controversial patch 693953dd4699887bd3f5bca2c3582b5fae1d6992 that explicitly called Calc to re-calculate rows after ODS load.
Comment 28 Justin L 2022-09-17 15:36:51 UTC
This is fairly easily fixed for XLS in sc/source/ui/docshell/docsh.cxx's ScDocShell::ConvertFrom by setting bSetRowHeights = true. Although that does every row in every sheet, I don't think the performance will be much worse than tracking each row during import - except perhaps for unused rows.
Comment 29 Justin L 2022-09-17 15:46:23 UTC
I confirm that Sample-C.xls from comment 26 is another good example for this bug report.
Comment 30 Justin L 2022-09-19 21:18:29 UTC
(In reply to Justin L from comment #28)
> This is fairly easily fixed for XLS
The code change is here. https://gerrit.libreoffice.org/c/core/+/140199

It shows good potential, with only one unit test failing, and that at least partly is because XLSX also needs a fix like this.
But even so, empty rows are calculated differently, so the unit test still fails when XLSX is also "fixed".

For now, I leave this as a code pointer only.
Comment 31 Justin L 2022-09-19 22:04:45 UTC
This bSetRowHeights was true earlier (and called UpdateAllRowHeights), until
commit c3ce9a34e710538aee37def65d0077af18140bcb
Author: Daniel Rentz on Tue Oct 23 13:51:33 2001 +0000
    #93255# speed up chart import

bSetRowHeights started calling the brand new ScDocRowHeightUpdater with
commit 0cfa5a12cc75e95f26fb6d8b348045bf8ba9bdd1
Author: Kohei Yoshida on Fri Oct 1 22:30:59 2010 -0400
    Ported calc-perf-import-dbf-sc.diff from ooo-build

    This speeds up import of dBase (dbf) files 4-5 times.

and as noted in comment 27, ScDocRowHeightUpdater is also used with ODS import.
Comment 32 Justin L 2022-09-20 14:18:43 UTC
*** Bug 39486 has been marked as a duplicate of this bug. ***
Comment 33 Justin L 2022-09-21 14:23:41 UTC
(In reply to Justin L from comment #30)
> But even so, empty rows are calculated differently
That might be inevitable. The difference that I saw was border spacing (.71mm in XLS and .35mm in .XLSX). AFAICS, these are unchangeable, built-in defaults although I haven't found the code for that yet. Bug 140185 talks about this.
Comment 34 Justin L 2022-09-21 19:27:00 UTC
(In reply to Justin L from comment #33)
> I haven't found the code for that yet.
sc/source/filter/excel/xistyle.cxx's XclImpXF::CreatePattern
    // Excel's cell margins are different from Calc's default margins.
    SvxMarginItem aItem(40, 40, 40, 40, ATTR_MARGIN);

[Excel here meaning binary excel. oox matches Calc's default margins and so doesn't need to do anything different.]
Comment 35 Justin L 2024-03-12 18:19:24 UTC
Is this not being called for XLS?
void ImportExcel::AdjustRowHeight()
{
    /*  Speed up chart import: import all sheets without charts, then
        update row heights (here), last load all charts -> do not any longer
        update inside of ScDocShell::ConvertFrom() (causes update of existing
        charts during each and every change of row height). */
    if( ScModelObj* pDocObj = GetDocModelObj() )
        pDocObj->UpdateAllRowHeights();
}