protobi / js-xlsx

XLSX / XLSM / XLSB (Excel 2007+ Spreadsheet) / ODS parser and writer
http://oss.sheetjs.com/js-xlsx
Other
826 stars 416 forks source link

"That command cannot be used on multiple selections" #26

Closed jlhjr closed 8 years ago

jlhjr commented 8 years ago

I get the below error message when I attempt to add a new worksheet to / copy cells from an Excel 2007 file generated in browser. The issue does not seem to occur with the original SheetJS version, but then I lose the styling options that make this version so great!

image

Here is an example: https://jsfiddle.net/jlhjr/0f8L9yfu/1/. Am I doing something obviously wrong?

Note that a workaround is to do a "Save As" of the temporary file, which must cause Excel to fix the underlying issue.

Thank you!

pietersv commented 8 years ago

@jlhjr Wow, great catch. The issue can be replicated in OSX Office 2011 and Office 365 for files generated in the browser and in Node, so this is a general issue and certainly not your doing.

The issue seems to be resolvable by either commenting out line 312 of bits/67_wsxml.js (and thus line 7845 of xlsx.js) or changing it from

tabSelected: opts.tabSelected === undefined ? '1' :  opts.tabSelected,

to

tabSelected: opts.tabSelected || undefined,

Commenting it out of course will disable the new feature to set a particular tab as selected, but likely that's minor.

Comparing the file that this branch of js-xlsx generates to the one that Excel creates after opening and resaving, it's not clear to me yet why this change should matter, as both have

<sheetView tabSelected="1" workbookViewId="0">

_Update:_ On dev queue. No date yet assigned

jlhjr commented 8 years ago

Thanks for the tip. Commenting out that line certainly fixes the issue, but instead I decided to simply pass tabSelected = false in the workbook opts to avoid the problem entirely.

My initial sense is that for

<sheetView tabSelected="1">

to work, there must be a corresponding

<bookViews>
    <workbookView activeTab="0" />
</bookViews>

workbookView element in workbook.xml. The OpenXML SDK hints at why that might be:

tabSelected (Sheet Tab Selected)

Flag indicating whether this sheet is selected. When only 1 sheet is selected and active, this value should be in synch with the activeTab value. In case of a conflict, the Start Part setting wins and sets the active sheet tab.

Multiple sheets can be selected, but only one sheet shall be active at one time.

The possible values for this attribute are defined by the W3C XML Schema boolean datatype.

Just speculating here, but one of the things that resaving the file appears to do is add the necessary workbookView element missing from the browser (or Node) generated file. The activeTab property of this element can specify the sheet to show on file open, but the default is to show the first tab.

So, in the short term, I might suggest that tabSelected default to '0' if undefined instead of '1' (i.e. the line you mention above). To truly implement pre-selected tabs, it seems we need to also specify an active tab at the workbook level or allow it to default to the first tab.

trcollinson commented 8 years ago

My users were also having this problem with workbooks created by this version with the default tabSelected equal to true. They also noticed that when they would delete some sheets from a workbook, Excel would hang and quit unexpected. Setting tabSelected = false in the worksheet opts solved the problem.

pietersv commented 8 years ago

@jlhjr @trcollinson Thanks! For now, have changed this to set tabSelected="0" unless otherwise specified. Aim to add < bookViews> later.

Because tests don't yet work on Node 4+, pushed these without a comprehensive test run.