Bug 56887 - [PATCH] Remove names pointing at a sheet when the sheet is deleted
Summary: [PATCH] Remove names pointing at a sheet when the sheet is deleted
Status: NEEDINFO
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.11-dev
Hardware: PC Linux
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2014-08-26 11:33 UTC by David North
Modified: 2016-06-11 03:12 UTC (History)
0 users



Attachments
Patch: remove names referencing removed sheet (2.47 KB, patch)
2014-08-26 11:33 UTC, David North
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David North 2014-08-26 11:33:51 UTC
Created attachment 31942 [details]
Patch: remove names referencing removed sheet

Summary: [PATCH] Remove names pointing at a sheet when the sheet is deleted

See attached patch.

If a Name is scoped to a particular worksheet, then onSheetDelete removes the name when the sheet is removed.

If the Name is however global, but points at the sheet being deleted (so that Name.getSheetName refers to the deleted sheet), the Name is not removed.

It'd be preferable if the Name was also removed in this case. Patch including unit test attached.
Comment 1 Nick Burch 2014-08-26 11:51:19 UTC
Could you confirm what the behaviour is in Microsoft Excel when you delete a sheet with a global name pointing to it?
Comment 2 David North 2014-08-26 12:44:16 UTC
Excel leaves the name intact but replaces references to DeletedSheetName with #REF!

I had previously thought that POI's current behaviour was leading to workbooks which Excel refused to open without repairing. However, I'm struggling to reproduce that, so perhaps this should be left as-is.
Comment 3 Nick Burch 2014-08-26 13:04:01 UTC
I'm somewhat minded to say we ought to do the same

That might also be what we already effectively do for HSSF, since that stores sheet references in formulas (like names) differently and by index

Could you tweak your unit test to try for .xls / hssf too, so we can verify what happens there?
Comment 4 Javen O'Neal 2016-06-11 03:12:06 UTC
From o.a.p.hssf.model.InternalWorkbook#removeSheet:
>> Within NameRecords, it's ok to have the formula
>>  part point at deleted sheets. It's also ok to
>>  have the ExternSheetNumber point at deleted
>>  sheets.
>> However, the sheet index must be adjusted, or
>>  excel will break. (Sheet index is either 0 for
>>  global, or 1 based index to sheet)

Looks like the Names cleanup may need to span HSSFWorkbook and InternalWorkbook since HSSFWorkbook maintains a data structure for HSSFNames.