Bug 45720

Summary: cloneSheet breaks autofilters
Product: POI Reporter: Antti Koskimäki <antti.koskimaki>
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Attachments: excel and sourcecode to reproduce bug
Biff dump of MSODRAWING before and after manual hex editing
excel with two sheets and source to reproduce the bug

Description Antti Koskimäki 2008-09-01 01:19:35 UTC
Created attachment 22505 [details]
excel and sourcecode to reproduce bug

Excel Autofilter does not survive through cloneSheet. Autofilter on original sheet does work, but on cloned sheet there is UI-component visible but it does not respond anymore.

Same behaviour on latest SVN (r690837) as well as all 3.0.2-FINAL and 3.1-FINAL.

Excel file + source for reproducing attached.

P.S. how badly does this relate to bug 35125... ???
Comment 1 Josh Micich 2008-09-03 12:58:00 UTC
I could reproduce this bug fairly easily.  One slight difference however, was that my Excel(2007) completely crashes when I try to sort using the autofilter in the newly copied sheet.

To help diagnose, I cloned the sheet in the input spreadsheet using Excel and ran BiffViewer on that file, and the POI generated file.

The first problem I noticed was a missing NameRecord (of the 'built-in' kind).  Another cosmetic problem was POI's choice of name for the new sheet.  Excel adds a space before the bracket e.g. "Test Clone (2)", which upsets the file offsets and causes false deltas when comparing the BiffViewer files.  If fixed both these problems in svn r691740. No junit for the built-in name record yet.

After that fix the next difference I tried working on was within the MSODRAWING record (in the second, copied sheet).  Unfortunately, POI does not interpret that record when reading from and existing file (BiffViewer does though), so I had to resort to hex-editing the POI generated XLS file.  After changing a few bytes (to make the POI generated second MSODRAWING record the same as the Excel generated one), the file loaded OK in Excel, and the autofilters worked fine.

I will attach the BiffViewer dump of the MSODRAWING records before and after manual hex editing.

Comment 2 Josh Micich 2008-09-03 13:08:07 UTC
Created attachment 22522 [details]
Biff dump of MSODRAWING before and after manual hex editing

biff45720-msoDrawing-bad.txt is a BiffViewer dump of the cloned drawing record (in the second sheet) as generated by POI with the example code of this bugzilla
biff45720-msoDrawing-good.txt is a BiffViewer dump of the same record after manual hex editing to make it the same as the one Excel produces.

These changes alone are enough to fix the POI generated XLS file so that the autofilters work OK in Excel.
Comment 3 Josh Micich 2008-09-03 13:18:03 UTC
By comparing the BiffViewer dumps, I can see that some internal IDs need adjusting, as well as an options flag.

There are probably a few steps required to complete this fix:
 1 - Make POI always interpret MSODRAWING records
 2 - Understand the the rules for updating the drawing IDs (do they just have to be  globally unique, or do they have to coordinate with other records?)
 3 - Implement those rules within HSSFWorkbook.cloneSheet()

Step 3 might be trivial, but Steps 1 and 2 definitely are not. Can someone who knows a bit about drawing records take a look at this?
Comment 4 Yegor Kozlov 2008-09-03 22:33:04 UTC
> 
> Step 3 might be trivial, but Steps 1 and 2 definitely are not. Can someone who
> knows a bit about drawing records take a look at this?
> 

I'm not sure if (1) is possible. Probably (2) would be enough. We can just traverse drawing records and update IDs when cloning sheets.

I will look into it and keep you informed about the status.

Yegor

Comment 5 Yegor Kozlov 2008-09-07 09:28:19 UTC
Fixed

If the cloned sheet has drawings, a new drawing group ID must be allocated. It was a crucial part of the bug.

See Workbook.cloneDrawings(Sheet sheet) for details.

Yegor
Comment 6 Antti Koskimäki 2008-09-07 22:49:51 UTC
(In reply to comment #5)
> Fixed
> 
> If the cloned sheet has drawings, a new drawing group ID must be allocated. It
> was a crucial part of the bug.

Thanks for quick responses.

I'll verify that you are getting closer, bug seemed to be fixed when tested with that simple example.

But, something is still wrong when there exists sheet(s) before the cloned one. 
Comment 7 Antti Koskimäki 2008-09-07 22:51:56 UTC
Created attachment 22537 [details]
 excel with two sheets and source to reproduce the bug

> But, something is still wrong when there exists sheet(s) before the cloned one. 

excel with two sheets and source to reproduce the bug
Comment 8 Yegor Kozlov 2008-09-08 06:33:57 UTC
Josh,

HSSFWorkbook.findExistingBuiltinNameRecordIdx does not find NameRecord if there is a sheet before the cloned one.
As result, the NameRecord is missing in the output. 

Run the code from the second attachment. The following "if" clause is not executed: 

int filterDbNameIndex = findExistingBuiltinNameRecordIdx(sheetIndex, NameRecord.BUILTIN_FILTER_DB);
if (filterDbNameIndex >=0) {
  ...
}

Yegor
Comment 9 Josh Micich 2008-09-08 13:00:23 UTC
(In reply to comment #8)

> HSSFWorkbook.findExistingBuiltinNameRecordIdx does not find NameRecord if there
> is a sheet before the cloned one.

Seemed to be a wrong interpretation of NamedRecord.getSheetNumber().  This bug would also have affected HSSFWorkbook.setRepeatingRowsAndColumns.  Fixed and junit added in svn r693221. Attachment (id=22537) seems to work OK now. 

Please re-open this bug if you find any other special case (of cloning sheets with autofilters) that is not handled.
Comment 10 Antti Koskimäki 2008-09-09 02:10:14 UTC
> Please re-open this bug if you find any other special case (of cloning sheets
> with autofilters) that is not handled.

Thanks. Got my real-world case functioning, so everything seems ok for me.