Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Number Format with "cellText: false, cellDates: true" broken #2349

Open
woshiguabi opened this issue Aug 11, 2021 · 10 comments
Open

Custom Number Format with "cellText: false, cellDates: true" broken #2349

woshiguabi opened this issue Aug 11, 2021 · 10 comments

Comments

@woshiguabi
Copy link

File

sheetjs-bug.xlsx

Problem

Date column is standard date format, Custom Date column is custom format
image

image

when XLSX.read set cellDates: true

if(opts.cellDates && do_format && p.t == 'n' && SSF.is_date(SSF._table[fmtid])) { p.t = 'd'; p.v = numdate(p.v); }

fmtid is 30, and SSF.is_date got undefined so broken
image

Discussion

  1. SSF.is_date validate input and return false for unexpected input.
  2. use safe_is_date like safe_format with SSFImplicit table, and add more implicit format.
  3. not broken whole worksheet, because workbook.Sheets still includes the broken worksheet, just remove the broken cell.
@SheetJSDev
Copy link
Contributor

ECMA-376 (XLSX) does not specify a default for code 30, hence nothing is in the SSFImplicit table. It further states:

When values not present in the lists below are used, the behavior is implementation-defined.

There are some locale-specific overrides for that particular code:

locale fmt 30
zh_TW m/d/yy
zh_CN m"-"d"-"yy
ja_JP m/d/yy
ko_KR mm"-"dd"-"yy

XLSB (2.5.76 Ifmt) refers to XLSX

XLS (2.4.126 Format) is even stranger, not prescribing a default for 30 and stating

The value of ifmt.ifmt MUST be a value within one of the following ranges or within 383 to 392.

  • 5 to 8
  • 23 to 26
  • 41 to 44
  • 63 to 66
  • 164 to 382

Out of curiosity, can you repeat that process and save the file in a few different formats, namely "Excel 97-2003 Workbook (.xls)" and "SYLK (Symbolic Link) (.slk)"?

@woshiguabi
Copy link
Author

Out of curiosity, can you repeat that process and save the file in a few different formats, namely "Excel 97-2003 Workbook (.xls)" and "SYLK (Symbolic Link) (.slk)"?

For *.xls, SSF.is_date(SSF._table[fmtid] || String(fmtid)) will skip fmt 30 and read as normal number

if(opts.cellDates && fmtid && p.t == 'n' && SSF.is_date(SSF._table[fmtid] || String(fmtid))) {

image


For *.slk, the behavior is opposite to *.xls.
Custom date format can be recognized but normal date format cant.
image

@woshiguabi
Copy link
Author

ECMA-376 (XLSX) does not specify a default for code 30, hence nothing is in the SSFImplicit table. It further states:

When values not present in the lists below are used, the behavior is implementation-defined.

Certainly, ECMA-376 not includes code 30, number format to string should be implementation-defined.
I just considering whether to add these special cases when cellDates: true, these values should be converted to Date .

@SheetJSDev
Copy link
Contributor

We'd have to test across a few more locales. If Excel is always treating format code 30 as the default date format, we can just add it to SSFImplicit.

Can you share the SLK file? You may need to compress as ZIP and attach.

@woshiguabi
Copy link
Author

😉Request for an extra help, I want to know which custom format code should be recognized as Date, is there any specs to read ?
Many thanks!

@woshiguabi
Copy link
Author

We'd have to test across a few more locales. If Excel is always treating format code 30 as the default date format, we can just add it to SSFImplicit.

Can you share the SLK file? You may need to compress as ZIP and attach.

sheetjs-bug.slk.zip

@SheetJSDev
Copy link
Contributor

Thanks!

The function is a stripped down version of the tokenizer https://github.com/SheetJS/ssf/blob/master/bits/81_fmttype.js#L3

It looks for the following date tokens:

  • B1 and B2 (calendar system)
  • Y and E (year and era)
  • M, D, H, S (month/minute, day, hour, second)
  • A/P, AM/PM, 上午/下午 (12-hour indicator)

@SheetJSDev
Copy link
Contributor

Can you check what version of the library you are using (console.log(XLSX.version)) ?
With 0.17.0 setting cellDates: true generates dates for every cell:

> require("xlsx").readFile("sheetjs-bug.xlsx", {cellNF: true, cellDates: true}).Sheets.Sheet1
...
  A2: { t: 'd', v: 2021-08-10T04:00:00.000Z, w: '8/10/21' },
  B2: { t: 'd', v: 2021-08-10T04:00:00.000Z, w: '8/10/21' },
  A3: { t: 'd', v: 2021-08-11T04:00:00.000Z, w: '8/11/21' },
  B3: { t: 'd', v: 2021-08-11T04:00:00.000Z, w: '8/11/21' },
  A4: { t: 'd', v: 2021-08-12T04:00:00.000Z, w: '8/12/21' },
  B4: { t: 'd', v: 2021-08-12T04:00:00.000Z, w: '8/12/21' },
  A5: { t: 'd', v: 2021-08-13T04:00:00.000Z, w: '8/13/21' },
  B5: { t: 'd', v: 2021-08-13T04:00:00.000Z, w: '8/13/21' },
...

With cellNF: true it looks like the format for cell B2 is missed (so something is still incorrect):

  A2: { t: 'n', v: 44418, z: 'm/d/yy', w: '8/10/21' },
  B2: { t: 'n', v: 44418, z: undefined, w: '8/10/21' },
  A3: { t: 'n', v: 44419, z: 'm/d/yy', w: '8/11/21' },

@woshiguabi
Copy link
Author

Can you check what version of the library you are using (console.log(XLSX.version)) ?

image

My options:

XLSX.read(ev.data.arrayBuffer, {
    type: 'array',
    cellFormula: false,
    cellHTML: false,
    cellText: false,
    cellStyles: true,
    cellDates: true,
    sheetRows: 100000,
    WTF: true,
}),

Addtional: I'm using xlsx in Worker.

@woshiguabi
Copy link
Author

woshiguabi commented Aug 11, 2021

Can you check what version of the library you are using (console.log(XLSX.version)) ?

image

My options:

XLSX.read(ev.data.arrayBuffer, {
    type: 'array',
    cellFormula: false,
    cellHTML: false,
    cellText: false,
    cellStyles: true,
    cellDates: true,
    sheetRows: 100000,
    WTF: true,
}),

Addtional: I'm using xlsx in Worker.

Ok finally i find cellText: false will cause this error.

cellText: true

// SSF._table
{
    "0": "General",
    "1": "0",
    "2": "0.00",
    "3": "#,##0",
    "4": "#,##0.00",
    "9": "0%",
    "10": "0.00%",
    "11": "0.00E 00",
    "12": "# ?/?",
    "13": "# ??/??",
    "14": "m/d/yy",
    "15": "d-mmm-yy",
    "16": "d-mmm",
    "17": "mmm-yy",
    "18": "h:mm AM/PM",
    "19": "h:mm:ss AM/PM",
    "20": "h:mm",
    "21": "h:mm:ss",
    "22": "m/d/yy h:mm",
    "30": "m/d/yy", // added fmtid 30 in _table
    "37": "#,##0 ;(#,##0)",
    "38": "#,##0 ;[Red](#,##0)",
    "39": "#,##0.00;(#,##0.00)",
    "40": "#,##0.00;[Red](#,##0.00)",
    "45": "mm:ss",
    "46": "[h]:mm:ss",
    "47": "mmss.0",
    "48": "##0.0E 0",
    "49": "@",
    "56": "\"上午/下午 \"hh\"時\"mm\"分\"ss\"秒 \"",
    "176": "[$-F800]dddd\\,\\ mmmm\\ dd\\,\\ yyyy"
}

cellText: false

// SSF._table
{
    "0": "General",
    "1": "0",
    "2": "0.00",
    "3": "#,##0",
    "4": "#,##0.00",
    "9": "0%",
    "10": "0.00%",
    "11": "0.00E 00",
    "12": "# ?/?",
    "13": "# ??/??",
    "14": "m/d/yy",
    "15": "d-mmm-yy",
    "16": "d-mmm",
    "17": "mmm-yy",
    "18": "h:mm AM/PM",
    "19": "h:mm:ss AM/PM",
    "20": "h:mm",
    "21": "h:mm:ss",
    "22": "m/d/yy h:mm",
    "37": "#,##0 ;(#,##0)",
    "38": "#,##0 ;[Red](#,##0)",
    "39": "#,##0.00;(#,##0.00)",
    "40": "#,##0.00;[Red](#,##0.00)",
    "45": "mm:ss",
    "46": "[h]:mm:ss",
    "47": "mmss.0",
    "48": "##0.0E 0",
    "49": "@",
    "56": "\"上午/下午 \"hh\"時\"mm\"分\"ss\"秒 \"",
    "176": "[$-F800]dddd\\,\\ mmmm\\ dd\\,\\ yyyy"
}

"27": 'm/d/yy', "28": 'm/d/yy', "29": 'm/d/yy', "30": 'm/d/yy', "31": 'm/d/yy',

if(!opts || opts.cellText !== false) try {
if(SSF._table[fmtid] == null) SSF.load(SSFImplicit[fmtid] || "General", fmtid);

SSFImplicit already includes fmt30, but when cellText: false, it won't be used.

@woshiguabi woshiguabi changed the title Custom Number Format with "cellDates: true" broken Custom Number Format with "cellText: false, cellDates: true" broken Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants