last column of spreadsheet is lost if first column is blank #37

Closed
opened 2013-10-28 19:56:47 +00:00 by mitchellsundt · 2 comments
mitchellsundt commented 2013-10-28 19:56:47 +00:00 (Migrated from github.com)

Two bugs in parseSheet():

(1) the dimension test logic always fails, so the sheet dimension is never set.

(2) the column-index logic in parseSheet() is wrong -- it assumes one-based, but the cells.forEach() uses zero-based, so the -1 corrections are not appropriate.

Patch:

diff -r ac083a41a811 xlsxconverter/WebContent/js-xlsx/xlsx.js
--- a/xlsxconverter/WebContent/js-xlsx/xlsx.js  Mon Oct 28 12:42:35 2013 -0700
+++ b/xlsxconverter/WebContent/js-xlsx/xlsx.js  Mon Oct 28 12:44:41 2013 -0700
@@ -533,7 +533,7 @@

    /* 18.3.1.35 dimension CT_SheetDimension ? */
    var ref = data.match(/<dimension ref="([^"]*)"\s*\/>/);
-   if(ref && ref.indexOf(":") !== -1) s["!ref"] = ref[1];
+   if(ref && ref.length == 2 && ref[1].indexOf(":") !== -1) s["!ref"] = ref[1];

    var refguess = {s: {r:1000000, c:1000000}, e: {r:0, c:0} };
    var q = ["v","f"];
@@ -552,8 +552,9 @@
        var cells = x.substr(x.indexOf('>')+1).split(/<c/);
        cells.forEach(function(c, idx) { if(c === "" || c.trim() === "") return;
            c = "<c" + c;
-           if(refguess.s.c > idx - 1) refguess.s.c = idx - 1;
-           if(refguess.e.c < idx - 1) refguess.e.c = idx - 1;
+           // idx is already zero-based
+           if(refguess.s.c > idx) refguess.s.c = idx;
+           if(refguess.e.c < idx) refguess.e.c = idx;
            var cell = parsexmltag((c.match(/<c[^>]*>/)||[c])[0]); delete cell[0];
            var d = c.substr(c.indexOf('>')+1);
            var p = {};
@@ -600,7 +601,8 @@
            s[cell.r] = p;
        });
    });
-   if(!s["!ref"]) s["!ref"] = encode_range(refguess);
+   var rguess = encode_range(refguess);
+   if(!s["!ref"]) s["!ref"] = rguess;
    return s;
 }

Two bugs in parseSheet(): (1) the dimension test logic always fails, so the sheet dimension is never set. (2) the column-index logic in parseSheet() is wrong -- it assumes one-based, but the cells.forEach() uses zero-based, so the -1 corrections are not appropriate. Patch: ``` diff -r ac083a41a811 xlsxconverter/WebContent/js-xlsx/xlsx.js --- a/xlsxconverter/WebContent/js-xlsx/xlsx.js Mon Oct 28 12:42:35 2013 -0700 +++ b/xlsxconverter/WebContent/js-xlsx/xlsx.js Mon Oct 28 12:44:41 2013 -0700 @@ -533,7 +533,7 @@ /* 18.3.1.35 dimension CT_SheetDimension ? */ var ref = data.match(/<dimension ref="([^"]*)"\s*\/>/); - if(ref && ref.indexOf(":") !== -1) s["!ref"] = ref[1]; + if(ref && ref.length == 2 && ref[1].indexOf(":") !== -1) s["!ref"] = ref[1]; var refguess = {s: {r:1000000, c:1000000}, e: {r:0, c:0} }; var q = ["v","f"]; @@ -552,8 +552,9 @@ var cells = x.substr(x.indexOf('>')+1).split(/<c/); cells.forEach(function(c, idx) { if(c === "" || c.trim() === "") return; c = "<c" + c; - if(refguess.s.c > idx - 1) refguess.s.c = idx - 1; - if(refguess.e.c < idx - 1) refguess.e.c = idx - 1; + // idx is already zero-based + if(refguess.s.c > idx) refguess.s.c = idx; + if(refguess.e.c < idx) refguess.e.c = idx; var cell = parsexmltag((c.match(/<c[^>]*>/)||[c])[0]); delete cell[0]; var d = c.substr(c.indexOf('>')+1); var p = {}; @@ -600,7 +601,8 @@ s[cell.r] = p; }); }); - if(!s["!ref"]) s["!ref"] = encode_range(refguess); + var rguess = encode_range(refguess); + if(!s["!ref"]) s["!ref"] = rguess; return s; } ```
Niggler commented 2013-10-28 20:31:11 +00:00 (Migrated from github.com)

@mitchellsundt confirm that this works

@mitchellsundt confirm that this works
mitchellsundt commented 2013-10-28 23:57:28 +00:00 (Migrated from github.com)

Confirmed

On Mon, Oct 28, 2013 at 1:31 PM, Niggler notifications@github.com wrote:

@mitchellsundt https://github.com/mitchellsundt confirm that this works


Reply to this email directly or view it on GitHubhttps://github.com/Niggler/js-xlsx/issues/37#issuecomment-27253149
.

Mitch Sundt
Software Engineer
University of Washington
mitchellsundt@gmail.com

Confirmed On Mon, Oct 28, 2013 at 1:31 PM, Niggler notifications@github.com wrote: > @mitchellsundt https://github.com/mitchellsundt confirm that this works > > — > Reply to this email directly or view it on GitHubhttps://github.com/Niggler/js-xlsx/issues/37#issuecomment-27253149 > . ## Mitch Sundt Software Engineer University of Washington mitchellsundt@gmail.com
Sign in to join this conversation.
No Milestone
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sheetjs/sheetjs#37
No description provided.