Skip to content

Commit

Permalink
* Handle "bare" .xlsx <r> and <c> tags.
Browse files Browse the repository at this point in the history
  An example of such an out-of-spec sheet XML snippet (provided by @EarlJS):

  <sheetData>
    <row s="1199" customFormat="1">
      <c s="1259" t="s">
        <v>1637</v>
      </c>
    </row>
  </sheetData>
  • Loading branch information
audreyt committed Sep 29, 2014
1 parent dbf2be1 commit abf4762
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions static/xlsx.js
Expand Up @@ -2943,6 +2943,7 @@ return function parse_ws_xml_data(sdata, s, opts, guess) {
var tag;
var sstr;
var fmtid = 0, fillid = 0, do_format = Array.isArray(styles.CellXf), cf;
var implicit_row = -1;
for(var marr = sdata.split(rowregex), mt = 0, marrlen = marr.length; mt != marrlen; ++mt) {
x = marr[mt].trim();
var xlen = x.length;
Expand All @@ -2951,16 +2952,19 @@ return function parse_ws_xml_data(sdata, s, opts, guess) {
/* 18.3.1.73 row CT_Row */
for(ri = 0; ri < xlen; ++ri) if(x.charCodeAt(ri) === 62) break; ++ri;
tag = parsexmltag(x.substr(0,ri), true);
var tagr = parseInt(tag.r, 10);
++implicit_row;
var tagr = parseInt(tag.r, 10) || implicit_row;
if(opts.sheetRows && opts.sheetRows < tagr) continue;
if(guess.s.r > tagr - 1) guess.s.r = tagr - 1;
if(guess.e.r < tagr - 1) guess.e.r = tagr - 1;

/* 18.3.1.4 c CT_Cell */
cells = x.substr(ri).split(cellregex);
var implicit_col = -1;
for(ri = 1, cellen = cells.length; ri != cellen; ++ri) {
x = cells[ri].trim();
if(x.length === 0) continue;
++implicit_col;
cref = x.match(rregex); idx = ri; i=0; cc=0;
x = "<c " + x;
if(cref !== null && cref.length === 2) {
Expand Down Expand Up @@ -3021,7 +3025,7 @@ return function parse_ws_xml_data(sdata, s, opts, guess) {
}
}
safe_format(p, fmtid, fillid, opts);
s[tag.r] = p;
s[tag.r || encode_cell({ r: implicit_row, c: implicit_col })] = p;
}
}
}; })();
Expand Down

14 comments on commit abf4762

@audreyt
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @SheetJS — I wonder if you'd accept this as a pullreq? This out-of-spec XML does open in LibreOffice and (presumably) Excel, so it's at least supported by some implementations.

@SheetJSDev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@audreyt Do you have a sample file?

@audreyt
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please find a minimal example at: http://audreyt.org/tmp/sample.xlsx which opens fine in LibreOffice but not Numbers.app.

The only difference is the missing r attributes from row and c tags.

@SheetJSDev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@audreyt strangely, neither ECMA-376 nor [MS-XLSX] describe how an optional row should be handled. The XSD for the row element clearly says that the field is optional. I suspect it is intended to increment from the last seen row (implicit_row / implicit_col should take into account the row/col of the last cell, even if it had a row/col specified).

To test this out, can you create another file where cells A1, B2, B4, D4, E4, A5, E5, and F5 are filled?

@SheetJSDev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@audreyt @EarlJS what version of libreoffice are you using? With the following version I see the right output:

Version: 4.1.4.2
Build ID: 0a0440ccc0227ad9829de5f46be37cfb6edcf72

    <row collapsed="false" customFormat="false" customHeight="false" hidden="false" ht="12.1" outlineLevel="0" r="1">
      <c r="A1" s="0" t="n">
        <v>1</v>
      </c>

@audreyt
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LibreOffice does not generate such sheets; it just accepts them; I think @EarlJS uses some other XLSX-generating tool that conforms to XSD but not to ECMA.

@EarlJS
Copy link

@EarlJS EarlJS commented on abf4762 Sep 30, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SheetJSDev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EarlJS as described earlier can you generate a file with cells A1, B2, B4, D4, E4, A5, E5, and F5? I want to see if the program uses implicit rows or columns when dealing with gaps

@EarlJS
Copy link

@EarlJS EarlJS commented on abf4762 Sep 30, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SheetJSDev
Copy link

@SheetJSDev SheetJSDev commented on abf4762 Sep 30, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EarlJS
Copy link

@EarlJS EarlJS commented on abf4762 Sep 30, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SheetJSDev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EarlJS thanks for doing this!

As I suspected, it uses the implicit row/col even with gaps. @audreyt your proposed patch doesn't address this case (implicit_row has to be updated when you see the "r" in a row element). I'll put in a patch to js-xlsx.

@EarlJS
Copy link

@EarlJS EarlJS commented on abf4762 Sep 30, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SheetJSDev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EarlJS @audreyt xlsx v0.7.11 has a better fix. Thanks for reaching out!

Please sign in to comment.