ODS : Unsupported value type 'error' #548

New Issue

2017-02-04T06:20:40Z

lgodard commented

2017-02-04 06:20:40 +00:00

(Migrated from github.com)

Hi

following the #546 issue resolution, i confirm that the second exposed case undefined is solved

nevertheless, the first problem error remains

Error: Unsupported value type error
{"t":"error","v":null}

hacking ods.js:329 as

		if(q.t === 'string' || !q.t || q.t === 'error') {

show no more problem

Would it be possible to add this in a future xlsx version ?

Hi following the #546 issue resolution, i confirm that the second exposed case `undefined` is solved nevertheless, the first problem `error` remains ``` Error: Unsupported value type error {"t":"error","v":null} ``` hacking `ods.js:329` as ``` if(q.t === 'string' || !q.t || q.t === 'error') { ``` show no more problem Would it be possible to add this in a future `xlsx` version ?

SheetJSDev commented

2017-02-04 06:44:06 +00:00

(Migrated from github.com)

@lgodard thanks for following up! I tried to reproduce the issue in Excel 2016 by setting the cell A1 to =0/0, which generates a #DIV/0! error in Excel, and saving to OpenDocument Spreadsheet. This actually works as expected, and the file works well.

As it turns out, Excel does something bizarre here. It is actually generating a float value rather than an error value, which explains why the error wouldn't have been tripped. The error can be reproduced by following the same procedure in LibreOffice.

I have a rough sense for what needs to be fixed, but we need to enumerate the errors and make sure that LO is consistent.

@lgodard thanks for following up! I tried to reproduce the issue in Excel 2016 by setting the cell A1 to `=0/0`, which generates a `#DIV/0!` error in Excel, and saving to OpenDocument Spreadsheet. This actually works as expected, and the file works well. As it turns out, Excel does something bizarre here. It is actually generating a float value rather than an error value, which explains why the error wouldn't have been tripped. The error can be reproduced by following the same procedure in LibreOffice. I have a rough sense for what needs to be fixed, but we need to enumerate the errors and make sure that LO is consistent.

lgodard commented

2017-02-04 06:58:49 +00:00

(Migrated from github.com)

Hi,

Thanks a lot for your interrest

i think the main ods problem for error is in parsing and namespaces; I do not think it is related to any legal error calculation

looking at the xml that leads to an error type

     Error: Unsupported value type error
{"t":"error","v":null}
<table:table-cell table:style-name="ce56" table:formula="of:=code_:pj" office:value-type="string" office:string-value="" calcext:value-type="error">,,table:,table-cell

one can see 2 value-type definitions : office:value-type="string" and calcext:value-type="error"
the parser returns second one

IIUC, calcext: namespace comes from libreoffice as a future opendocument official evolution, but strictly speaking, out of the scope of odf 1.2 norm version

So, there is 2 possibilities

handle error as a legal value-type (less intrusive code as proposed)
a more strict parser that do not take care of calcextnamespace (more intrusive, maybe side effects)
same as 2 but with an optional mode that handles calcextnamespace (same as current behaviour)

one can step progressively on them

I'll try to build an ods file that reproduces the calcext:error problem, but can't promise i'll succeed

Btw, thanks a lot for your response and feel free to ask if any help/test needed

Laurent

Hi, Thanks a lot for your interrest i think the main ods problem for `error` is in parsing and namespaces; I do not think it is related to any legal error calculation looking at the xml that leads to an error type ``` Error: Unsupported value type error {"t":"error","v":null} <table:table-cell table:style-name="ce56" table:formula="of:=code_:pj" office:value-type="string" office:string-value="" calcext:value-type="error">,,table:,table-cell ``` one can see 2 `value-type` definitions : `office:value-type="string"` and `calcext:value-type="error"` the parser returns second one IIUC, `calcext:` namespace comes from libreoffice as a future opendocument official evolution, but strictly speaking, out of the scope of odf 1.2 norm version So, there is 2 possibilities 1. handle `error` as a legal `value-type` (less intrusive code as proposed) 2. a more strict parser that do not take care of `calcext`namespace (more intrusive, maybe side effects) 3. same as 2 but with an optional mode that handles `calcext`namespace (same as current behaviour) one can step progressively on them I'll try to build an ods file that reproduces the `calcext:error` problem, but can't promise i'll succeed Btw, thanks a lot for your response and feel free to ask if any help/test needed Laurent

lgodard commented

2017-02-04 06:59:11 +00:00

(Migrated from github.com)

oups, not intended to close

SheetJSDev commented

2017-02-04 07:16:14 +00:00

(Migrated from github.com)

Thanks again for digging further! The more interesting question is "what is the right way to handle this cell?", and it's really not clear.

A) Treat it as a string cell (t='s'). In the parlance of the OASIS spec, LO generates an extended OpenDocument, so it is perfectly acceptable to ignore the calcext hints. But it seems like a waste.

B) Optimistically treat it as an error cell (t='e') and map back to Excel error codes or generate fake error code values for LO errors that have no Excel equivalent. This is great for reading, but it is unclear how to serialize errors that have no equivalent in Excel.

On a side note: it's unfortunate that office:value-type itself doesn't support error values, especially since Excel treats errors as a special value type.

Thanks again for digging further! The more interesting question is "what is the right way to handle this cell?", and it's really not clear. A) Treat it as a string cell (`t='s'`). In the parlance of the OASIS spec, LO generates an extended OpenDocument, so it is perfectly acceptable to ignore the `calcext` hints. But it seems like a waste. B) Optimistically treat it as an error cell (`t='e'`) and map back to Excel error codes or generate fake error code values for LO errors that have no Excel equivalent. This is great for reading, but it is unclear how to serialize errors that have no equivalent in Excel. On a side note: it's unfortunate that `office:value-type` itself doesn't support error values, especially since Excel treats errors as a special value type.

lgodard commented

2017-02-04 07:30:39 +00:00

(Migrated from github.com)

As a first shot, i would stick to A) and not try to make any guess on what the odf xml gives itself

reading the offending xml

<table:table-cell 
    table:style-name="ce56" 
    table:formula="of:=code_:pj" 
    office:value-type="string" 
    office:string-value="" 
    calcext:value-type="error">

one can see that table:formula is not a supported format
code_:pj is a named range but contains a : character which is not valid in LibreOffice for a long time (but was legal in the past)
i guess that the libreoffice team decided to keep the formula as given by the xml but deactived by the calcextnamespace

My understanding of this calcext:value-type="error"is

this cell would lead to an internal LibreOffice error, so we deactivate it, but keep its original content as coming from other third-party software (OpenOffice, hand crafted aso ...)

I'm actually trying to build a simpler file for unit testing. I let you know

regarding your error values, feel free to ask any ods file that would help you

Laurent

As a first shot, i would stick to A) and not try to make any guess on what the odf xml gives itself reading the offending xml ``` <table:table-cell table:style-name="ce56" table:formula="of:=code_:pj" office:value-type="string" office:string-value="" calcext:value-type="error"> ``` one can see that `table:formula` is not a supported format `code_:pj` is a named range but contains a `:` character which is not valid in LibreOffice for a long time (but was legal in the past) i guess that the libreoffice team decided to keep the formula as given by the xml but deactived by the `calcext`namespace My understanding of this `calcext:value-type="error"`is > this cell would lead to an internal LibreOffice error, so we deactivate it, but keep its original content as coming from other third-party software (OpenOffice, hand crafted aso ...) I'm actually trying to build a simpler file for unit testing. I let you know regarding your error values, feel free to ask any ods file that would help you Laurent

lgodard commented

2017-02-04 07:52:15 +00:00

(Migrated from github.com)

Hi again,

Here is a simplified file that reproduces the problem

I was half-wrong (so half-right ;-) ) on the interpretation
The formula marked in yellow in the file is the problem
it is invalid (marked as #NAME in libreoffice) but it was intended as this sheet is used as templating (and the formula programmatically modified in libreoffice)

So, one can do A) and maybe B) considering that any calcext error leads to a #NAME error but that may be implementation-dependant. (so i still would stick to A) )

If needed i can search in libreoffice source code if you need some details on the #NAME error building and give you code pointers (but i do not think it is needed)

warning : rename it removing .zip extension

calcext_error_NAME.ods.zip

Hi again, Here is a simplified file that reproduces the problem I was half-wrong (so half-right ;-) ) on the interpretation The formula marked in yellow in the file is the problem it is invalid (marked as #NAME in libreoffice) but it was intended as this sheet is used as templating (and the formula programmatically modified in libreoffice) So, one can do A) and maybe B) considering that any calcext error leads to a #NAME error but that may be implementation-dependant. (so i still would stick to A) ) If needed i can search in libreoffice source code if you need some details on the #NAME error building and give you code pointers (but i do not think it is needed) warning : rename it removing .zip extension [calcext_error_NAME.ods.zip](https://github.com/SheetJS/js-xlsx/files/752096/calcext_error_NAME.ods.zip)

SheetJSDev commented

2017-02-04 08:04:02 +00:00

(Migrated from github.com)

The error message is actually localized in the file:

<table:table-cell table:style-name="ce2" table:formula="of:=code_:pj" office:value-type="string" office:string-value="" calcext:value-type="error">
  <text:p>#NOM ?</text:p>
</table:table-cell>

I'm convinced that A) is better. We're hoping to make another push sometime this month or early March.

The error message is actually localized in the file: ``` <table:table-cell table:style-name="ce2" table:formula="of:=code_:pj" office:value-type="string" office:string-value="" calcext:value-type="error"> <text:p>#NOM ?</text:p> </table:table-cell> ``` I'm convinced that A) is better. We're hoping to make another push sometime this month or early March.

lgodard commented

2017-02-04 08:06:37 +00:00

(Migrated from github.com)

great !!!!
once solved, i'll can use xlsx deeply :)
thanks again

great !!!! once solved, i'll can use xlsx deeply :) thanks again

lgodard commented

2017-02-20 08:19:05 +00:00

(Migrated from github.com)

@SheetJSDev anything I can do to help finalizing this issue ?

SheetJSDev commented

2017-02-21 06:30:21 +00:00

(Migrated from github.com)

@lgodard there are 3 types of "extensions" based on my experimentation: loext ooext and calcext. They can be reproduced by saving files in open office and libre office either in the ODS format or the Flat ODS format. It is sufficient to reject any parameter override with namespace ending in ext.

The fix, which should be going in sometime by the end of the week, is to change the else clause of the parsexmltag function in ods.js as follows:

		else {
			var k = (j===5 && q.substr(0,5)==="xmlns"?"xmlns":"")+q.substr(j+1); // <-- generate the key
			if(z[k] && q.substr(j-3,3) == "ext") continue; // <-- do not let any extension override an existing value
			z[k] = v;
		}

@lgodard there are 3 types of "extensions" based on my experimentation: `loext` `ooext` and `calcext`. They can be reproduced by saving files in open office and libre office either in the ODS format or the Flat ODS format. It is sufficient to reject any parameter override with namespace ending in `ext`. The fix, which should be going in sometime by the end of the week, is to change the else clause of the parsexmltag function in ods.js as follows: ```js else { var k = (j===5 && q.substr(0,5)==="xmlns"?"xmlns":"")+q.substr(j+1); // <-- generate the key if(z[k] && q.substr(j-3,3) == "ext") continue; // <-- do not let any extension override an existing value z[k] = v; } ```

lgodard commented

2017-02-21 09:45:31 +00:00

(Migrated from github.com)

guy, you rock !

Sign in to join this conversation.