Bug 161599 - Extension is not handling formulas from MS Excel created files for all versions besides 7.3.7
Summary: Extension is not handling formulas from MS Excel created files for all versio...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.5.0.3 release
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:25.2.0 target:24.8.2
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-16 15:07 UTC by Emil
Modified: 2024-09-04 09:36 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Zip of two files, extension and sample file (70.51 KB, application/zip)
2024-06-16 15:07 UTC, Emil
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Emil 2024-06-16 15:07:17 UTC
Created attachment 194759 [details]
Zip of two files, extension and sample file

This sample extension implements one formula =INCREMENTBYONE(x), simple increment of the number x by one.

Ty reproduce:
1. Install 7.3.7 version of the LibreOffice
2. Install the extension
3. Open the attached "sample.xlsx" file

Expected Result: A1 -> 2
Actual Result: A1 -> 2

4. Install any other version ( I tried 7.4.7 and 24.2 ) of the LibreOffice
5. Install the extension 
6. Open the attached "sample.xlsx" file

Expected Result: A1 -> 2
Actual Result: A1 -> #NAME?


Attached is sample.xlsx, which was created by MS Excel.
Comment 1 Buovjaga 2024-08-22 05:52:22 UTC
I bibisected the change with linux-64-7.4 to ad15930e60b1e175d127022fd99fe71f140cbd88
Resolves: tdf#142293 Implement the temporariness of GetOpCodeMap()

The title of bug 142293 is "Spreadsheet functions implemented as AddIn may get saved with their programmatic name to OOXML Excel .xlsx instead of the function name". So this makes me think what you see is not a bug in itself, but you can fix it by recreating the xlsx or adjusting your extension. Can you check it?
Comment 2 Emil 2024-08-25 06:50:13 UTC Comment hidden (noise)
Comment 3 Buovjaga 2024-08-25 07:17:31 UTC Comment hidden (off-topic)
Comment 4 Mike Kaganski 2024-08-26 07:48:44 UTC
(In reply to Buovjaga from comment #1)
> I bibisected the change with linux-64-7.4 to
> ad15930e60b1e175d127022fd99fe71f140cbd88

So that is a backport commit, and also a combination of five commits. This is why bisection in post-branch-off commits is usually not helpful.

Could you please bisect in linux-64-7.5 (I assume it was the master then)? Thanks!
Comment 5 Buovjaga 2024-08-26 09:37:12 UTC
(In reply to Mike Kaganski from comment #4)
> (In reply to Buovjaga from comment #1)
> > I bibisected the change with linux-64-7.4 to
> > ad15930e60b1e175d127022fd99fe71f140cbd88
> 
> So that is a backport commit, and also a combination of five commits. This
> is why bisection in post-branch-off commits is usually not helpful.
> 
> Could you please bisect in linux-64-7.5 (I assume it was the master then)?
> Thanks!

Good point. Checking those commits directly, the exact change is 5aaa0cda74de4170972b7988d8fca16a560b8500
Resolves: tdf#142293 Implement the temporariness of GetOpCodeMap()
Comment 6 Mike Kaganski 2024-08-26 09:50:55 UTC
(In reply to Emil from comment #0)
> Attached is sample.xlsx, which was created by MS Excel.

In fact, it was created by Openpyxl; and opening it in MS Excel warns that "We found a problem with some content in 'sample.xlsx'. Do you want us to try to recover as much as we can?". Not really relevant to the problem, though.
Comment 7 Emil 2024-08-26 10:22:00 UTC
Dear Mike, 
This excel file can be opened my MS Excel without any errors, please see the attached recording. https://d.pr/v/MwheZw

Also, it is not really matter if the excel was created by MS excel or any other excel lib. The problem is Libreoffice allows to create custom functions which should be stored in the way it can be readable by any other excel processor. Libreoffice did it correctly until the version 7.3.7. Later versions store the function with the preamble, wich can't be read correctly by another excel processors which capable to process the same custom function.

I just made an example with the simple function to reproduce the bug, but some of the functions i implemented already supported by excel ( like XLOOKUP ) and so, but need to be added to Libreoffice support.
Comment 8 Mike Kaganski 2024-08-26 10:42:15 UTC
So indeed, that change is relevant. The add-in registers function 'COM.XEVALAI.XEVAL.OXT.INCREMENTBYONE', but prior to the change, it handled undecorated plain 'INCREMENTBYONE'. Now it only handles the fully qualified name, as intended.

So what should be the change in the add-in, to handle unqualified name? Eike, could you advise? Thank you!
Comment 9 Mike Kaganski 2024-08-26 10:43:36 UTC
(In reply to Emil from comment #7)

Please avoid unneeded posts. My comment 6 mentioned a fact, and noted that it wasn't really relevant for the problem. Your comment 7 only created noise. Thanks.
Comment 10 Eike Rathke 2024-08-27 16:54:44 UTC
Reading sample.xlsx I do not get a #NAME? error with any version (tried 25.2.0.0.alpha0+, 7.4.7.2.0+ and 24.2.6.1.0+) neither with the Archive.zip's sample.xlsx nor the sample.xlsx of xevalai.oxt. They both have COM.XEVALAI.XEVAL.OXT.INCREMENTBYONE(1) stored. Replacing that with INCREMENTBYONE(1) in xl/worksheets/sheet1.xml though yields #NAME? error. Saving (a working INCREMENTBYONE(1)) to .xlsx however it uses the full programmatic name. I can't even install the extension successfully in 7.3.7 or any earlier I tried (might be on my self-built side), so can't say if it really stored INCREMENTBYONE(1) instead of the full programmatic name.

What we maybe could do is if a (English) CompatibilityName was defined (which may even be locale-dependent, thanks to Excel's weirdos) in the .xcu, i.e. INCREMENTBYONE, to store that and also read it.

However, that could not be read by older LibreOffice versions that could read the full programatic name, so isn't ideal either. YMMV..

In any case we probably could recognize such CompatibilityName when reading from .xlsx as well (that mechanism originally was for .xls BIFF).


(CompatibilityName in addin.xcu would be to add

 <prop oor:name="CompatibilityName"><value xml:lang="en">INCREMENTBYONE</value></prop>

for example after the oor:name="Category" property).
Comment 11 Commit Notification 2024-08-27 19:10:03 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

Related: tdf#161599 ChildAccess::asValue() must return Reference<XInterface>

It will be available in 25.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.
Comment 12 Buovjaga 2024-08-27 19:58:16 UTC Comment hidden (off-topic)
Comment 13 Commit Notification 2024-08-28 05:07:14 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/61e11389f2bebf605fbebfc58ef4e345941bf907

Related: tdf#161599 ChildAccess::asValue() must return Reference<XInterface>

It will be available in 24.8.2.

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 Eike Rathke 2024-08-29 08:56:45 UTC Comment hidden (off-topic)
Comment 15 Eike Rathke 2024-08-29 10:14:59 UTC
Btw, I doubt that a file created by Excel ever would had written INCREMENTBYONE because function names not defined by OOXML get a _xlfn. prefix, so that should had been _xlfn.INCREMENTBYONE instead. For Excel Add-In functions it might even be a different one, not sure at the moment (Mike? on Cc).


(In reply to Emil from comment #7)
> This excel file can be opened my MS Excel without any errors, please see the
> attached recording. https://d.pr/v/MwheZw
That shows a =@INCREMENTBYONE(1) formula (note the leading @) and equally a #NAME? error.

> Also, it is not really matter if the excel was created by MS excel or any
> other excel lib.
I doubt that. As said, Excel would had added some prefix.

> The problem is Libreoffice allows to create custom
> functions which should be stored in the way it can be readable by any other
> excel processor. Libreoffice did it correctly until the version 7.3.7.
Unfortunately you did not provide a file actually stored by LO 7.3.7 so we could take a look. Or even a file saved by Excel.

> I just made an example with the simple function to reproduce the bug, but
> some of the functions i implemented already supported by excel ( like
> XLOOKUP ) and so, but need to be added to Libreoffice support.
XLOOKUP and others are implemented as of LO 24.8, see https://wiki.documentfoundation.org/ReleaseNotes/24.8#Calc
Comment 16 Mike Kaganski 2024-08-29 10:25:11 UTC
(In reply to Eike Rathke from comment #15)
> Btw, I doubt that a file created by Excel ever would had written
> INCREMENTBYONE because function names not defined by OOXML get a _xlfn.
> prefix, so that should had been _xlfn.INCREMENTBYONE instead. For Excel
> Add-In functions it might even be a different one, not sure at the moment
> (Mike? on Cc).

My Excel doesn't have INCREMENTBYONE function, so indeed the function in the attachment 194759 [details] is shown as a #NAME? error (and this time, I didn't get the warning I mentioned in comment 6 - possibly that was something about corruption of the file when I extracted it? Doesn't matter, because that was obviously my local problem). Re-saving the same file in Excel keeps the bare "<f ca="1">INCREMENTBYONE(1)</f>". Creating such a file in Excel from scratch gives the function name lowercase: "<f ca="1">incrementbyone(1)</f>".

I don't know what will happen when using an Excel extension defining a function, though.
Comment 17 Buovjaga 2024-08-29 10:55:09 UTC Comment hidden (off-topic)
Comment 18 Eike Rathke 2024-08-29 11:21:18 UTC Comment hidden (off-topic)
Comment 19 Commit Notification 2024-08-29 11:47:03 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5755b5515b8e97b638f2fb1f7553130760f12e94

Related: tdf#161599 pre-initialize ScCompiler ODFF and OOXML final OpCodeMap

It will be available in 25.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.
Comment 20 Eike Rathke 2024-08-29 11:54:21 UTC
(In reply to Mike Kaganski from comment #16)
> I don't know what will happen when using an Excel extension defining a
> function, though.
This is all confusing. The attached Add-In apparently was meant as an example but the original problem was about the XLOOKUP function implemented as Add-In, and that when saved to .xlsx should definitely be _xlfn.XLOOKUP instead of plain XLOOKUP. Calc is not prepared to save such from an arbitrary Add-In, not even with a compatibiblity name.

I think I can implement it as a general measure, but of course that will be superfluous for the XLOOKUP Add-In because XLOOKUP is now an internal function.
Comment 21 Buovjaga 2024-08-29 16:12:41 UTC Comment hidden (off-topic)
Comment 22 Commit Notification 2024-08-29 16:54:58 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6159c597b5e20db2cee919b116bb7a0b663ca9d7

Related: tdf#161599 Do not skip first of fallbacks of CompatibilityName locale

It will be available in 25.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.
Comment 23 Commit Notification 2024-08-29 18:31:13 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/59d2745d4ab0bb44d474bab56d3f06a8e1dfb788

Related: tdf#161599 Accept Add-In CompatibilityName when reading OOXML

It will be available in 25.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.
Comment 24 Commit Notification 2024-08-30 01:05:01 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/044980edc41544dd5973d5aa57f134d4bfe6d0e7

Resolves: tdf#161599 Write Add-In CompatibilityName (if any) to OOXML

It will be available in 25.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.
Comment 25 Eike Rathke 2024-08-30 02:09:07 UTC
With these changes it is possible to read and write a compatibility name that is defined in the extension's .xcu file. The defined name must be identical to what Excel (!) writes when writing out the function name. For the example here this likely would be

  <prop oor:name="CompatibilityName"><value xml:lang="en-US">_xlfn.INCREMENTBYONE</value></prop>

Note the "_xlfn." prefix (similar to the _xlfn.XLOOKUP mentioned earlier).

xml:lang can be "en-US" (preferred) or "en". In case neither is present, the first CompatibilityName with any other language tag is taken (this indeterministic behaviour is a legacy of existing implementation and kept for congruency), so always define a "en-US" or "en" CompatibilityName if there is any other for whatever reason. If no CompatibilityName is defined, the fully qualified function name is written, as before.
Comment 26 Commit Notification 2024-08-30 09:36:05 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/1700d33d8b4ce721a774b8fa894c42375acd6db9

Related: tdf#161599 Do not skip first of fallbacks of CompatibilityName locale

It will be available in 24.8.2.

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 27 Commit Notification 2024-09-03 07:02:06 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/4b0ff8ae2a8b9d3945cd0f23535c7ef354a9b3d4

Related: tdf#161599 Accept Add-In CompatibilityName when reading OOXML

It will be available in 24.8.2.

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 28 Commit Notification 2024-09-03 07:02:08 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/5a9f5931134a4062f90860f1dc76717d81a8d6b2

Resolves: tdf#161599 Write Add-In CompatibilityName (if any) to OOXML

It will be available in 24.8.2.

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 29 Commit Notification 2024-09-04 09:36:29 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

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

Related: tdf#161599 pre-initialize ScCompiler ODFF and OOXML final OpCodeMap

It will be available in 24.8.2.

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.