Bug 60541

Summary: Column collapsed attribute is incorrectly set when reading file
Product: POI Reporter: Christian Schroeder <cs>
Component: XSSFAssignee: POI Developers List <dev>
Status: NEW ---    
Severity: normal    
Priority: P2    
Version: 3.15-FINAL   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: Code to reproduce the erroneous behavior
Proposed patch from comment 0

Description Christian Schroeder 2017-01-02 20:02:30 UTC
Created attachment 34576 [details]
Code to reproduce the erroneous behavior

When reading an OOXML Excel document from a file, the "collapsed" attribute of any column is always set, even if the attribute was not set in the input file. If the attribute was not present in the input file, it will be set to "false". If the attribute was present in the input file, it will be set to "true", regardless of the actual value of the attribute. By saving and loading the document multiple times, the attribute will eventually have a true value, regardless of its initial state.

Example to reproduce this behavior is attached.

The error occurs in line 259 of org.apache.poi.xssf.usermodel.helpers.ColumnHelper. The existing line:

toCol.setCollapsed(fromCol.isSetCollapsed());

... should either be changed to:

toCol.setCollapsed(fromCol.getCollapsed());

... or removed at all.
Comment 1 Javen O'Neal 2017-01-03 02:21:46 UTC
Created attachment 34580 [details]
Proposed patch from comment 0

It looks like that line was added in r775731 for bug 46161 with the commit message "xssf support for setGroupColumnCollapsed() setGroupRowCollapsed()".

https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/ColumnHelper.java?annotate=775731&pathrev=775731#l221
> r636729       nick     public void setColumnAttributes(CTCol fromCol, CTCol toCol) {
> r1561435 kiwiwings         if(fromCol.isSetBestFit()) toCol.setBestFit(fromCol.getBestFit());
> ...
> r747895      yegor         if(fromCol.isSetCollapsed()) toCol.setCollapsed(fromCol.getCollapsed());
> ...
> r775731       nick         toCol.setCollapsed(fromCol.isSetCollapsed());
> r615258       nick     }

I'm in favor of deleting the unguarded toCol.setCollapsed, but it currently breaks TestXSSFSheet#setColumnGroupCollapsed.
This old unit test appears to have many problems, such as confusing isSetCollapsed with getCollapsed, assuming hidden is the same as collapsed, among many other problems. I am rewriting this unit test before making your changes to avoid regressions.