Bug 61905

Summary: Sheet.setActiveCell() does nothing
Product: POI Reporter: Davide Angelocola <davide.angelocola>
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 3.17-FINAL   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: active cell is A1, instead of E11
active cell is E11, as expected
proof
xls made with excel

Description Davide Angelocola 2017-12-14 14:54:22 UTC
Created attachment 35612 [details]
active cell is A1, instead of E11

The method setActiceCell seems to be not working on latest stable version. The same method on XSSF is working as expected. 

Please find a minimal reproducer below:


    @Test
    public void xlsx() throws IOException {
        XSSFWorkbook wb = new XSSFWorkbook();
        XSSFSheet sheet = wb.createSheet("new sheet");
        sheet.setActiveCell(new CellAddress("E11"));
        wb.write(new FileOutputStream("c:/temp/yyy.xlsx"));
        wb.close();
    }

    @Test
    public void xls() throws IOException {
        HSSFWorkbook wb = new HSSFWorkbook();
        HSSFSheet sheet = wb.createSheet("new sheet");
        sheet.setActiveCell(new CellAddress("E11"));
        wb.write(new FileOutputStream("c:/temp/yyy.xls"));
        wb.close();
    }
Comment 1 Davide Angelocola 2017-12-14 14:55:02 UTC
Created attachment 35613 [details]
active cell is E11, as expected
Comment 2 Dominik Stadler 2017-12-25 08:43:00 UTC
This works for me with LibreOffice on Linux, can you provide details about your environment?

Also does the following unit test pass?

    @Test
    public void test61905xlsx() throws IOException {
        Workbook wb = new XSSFWorkbook();
        checkActiveSheet(wb, XSSFITestDataProvider.instance);
        //wb.write(new FileOutputStream("/tmp/yyy.xlsx"));
        wb.close();
    }

    @Test
    public void test61905xls() throws IOException {
        Workbook wb = new HSSFWorkbook();
        checkActiveSheet(wb, HSSFITestDataProvider.instance);
        //wb.write(new FileOutputStream("/tmp/yyy.xls"));
        wb.close();
    }

    private void checkActiveSheet(Workbook wb, ITestDataProvider instance) throws IOException {
        Sheet sheet = wb.createSheet("new sheet");
        sheet.setActiveCell(new CellAddress("E11"));
        assertEquals("E11", sheet.getActiveCell().formatAsString());

        Workbook wbBack = instance.writeOutAndReadBack(wb);
        sheet = wbBack.getSheetAt(0);
        assertEquals("E11", sheet.getActiveCell().formatAsString());
        wbBack.close();
    }
Comment 3 Davide Angelocola 2017-12-27 09:22:35 UTC
I'm using Windows 10 Enterprise with Excel 2016 (MSO 16.0.8201.2207 32-bit). 
When I open the file yyy.xls with Excel the cell A1 is marked as active, instead of E11 (please see the attached screenshot).
Whereas when I open the file yyy.xlsx the cell E11 is marked as active, as expected.
Comment 4 Davide Angelocola 2017-12-27 09:24:02 UTC
Created attachment 35629 [details]
proof
Comment 5 Dominik Stadler 2018-04-19 07:27:35 UTC
On a quick look we do set the SelectionRecord correctly in HSSF, not sure why Excel does not use this. No obvious difference between the way POI sets the current cell and the way Excel stores it in the file.
Comment 6 Javen O'Neal 2018-04-19 09:33:53 UTC
Isn't the concept of the active cell and a selected cell different? Multiple calls can be selected, but only one can be active.

I thought I remembered the Javadocs talking about this differentiation, but this appears to be on marking a sheet as active or a set of sheets as selected in need workbook.
Comment 7 Javen O'Neal 2018-04-19 09:37:02 UTC
Davide, can you create 2 xlsx workbooks in Excel: one with A1 selected and the other with E11 selected, then unzip the xlsx workbooks and diff the xl/sheet1.xml files and xl/workbook.xml files?

Then compare the diff of POI's output. That might hint at what POI is doing differently.
Comment 8 Davide Angelocola 2018-04-19 15:35:59 UTC
Thanks for looking into this.

POI:
<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"> 
  <dimension ref="A1"/>
  <sheetViews>
    <sheetView workbookViewId="0" tabSelected="true">
      <selection activeCell="E11" sqref="E11"/>
    </sheetView>
  </sheetViews>
...

Excel:
<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"> 
  <dimension ref="A1:I30"/>
  <sheetViews>
    <sheetView workbookViewId="0">
      <selection activeCell="E11" sqref="E11"/>
    </sheetView>
 </sheetViews>

Perhaps E11 is outside dimension?
Comment 9 Mark Murphy 2018-04-20 14:54:13 UTC
It looks like there is some confusion over where the error is. By my reading the problem is with HSSF format though the initial description doesn't specify that. You are going to have to make .xls spreadsheets to compare against. I think there is a tool to look at the HSSF formats, not sure where it is though.
Comment 10 Nick Burch 2018-04-20 15:17:24 UTC
(In reply to Mark Murphy from comment #9)
> I think there is a tool to look at the HSSF formats, not sure where
> it is though.

BiffViewer is probably what you're thinking of - it lets you dump the record structures to compare
Comment 11 Andreas Beeker 2018-04-20 15:22:35 UTC
You could try my POI-Visualizer [1] and check the properties tab.

[1] https://github.com/kiwiwings/poi-visualizer
Comment 12 Davide Angelocola 2018-04-20 16:43:37 UTC
Created attachment 35886 [details]
xls made with excel

E11 is the active cell
Comment 13 Davide Angelocola 2018-04-20 16:46:08 UTC
Attached also a XLS made with MS Excel (Office 365).
Comment 14 Davide Angelocola 2018-04-20 16:53:38 UTC
A brief recap, just to avoid confusion:

- this bug is a basically an interoperability problem: when POI writes a file with a given active cell, MS excel ignores it;

- this bug happens only with XLS (see attached screenshot);

- XLSX is working fine.
Comment 15 Davide Angelocola 2018-04-20 17:37:31 UTC

(In reply to Dominik Stadler from comment #2)
> This works for me with LibreOffice on Linux, can you provide details about
> your environment?
> 
> Also does the following unit test pass?
> 
>     @Test
>     public void test61905xlsx() throws IOException {
>         Workbook wb = new XSSFWorkbook();
>         checkActiveSheet(wb, XSSFITestDataProvider.instance);
>         //wb.write(new FileOutputStream("/tmp/yyy.xlsx"));
>         wb.close();
>     }
> 
>     @Test
>     public void test61905xls() throws IOException {
>         Workbook wb = new HSSFWorkbook();
>         checkActiveSheet(wb, HSSFITestDataProvider.instance);
>         //wb.write(new FileOutputStream("/tmp/yyy.xls"));
>         wb.close();
>     }
> 
>     private void checkActiveSheet(Workbook wb, ITestDataProvider instance)
> throws IOException {
>         Sheet sheet = wb.createSheet("new sheet");
>         sheet.setActiveCell(new CellAddress("E11"));
>         assertEquals("E11", sheet.getActiveCell().formatAsString());
> 
>         Workbook wbBack = instance.writeOutAndReadBack(wb);
>         sheet = wbBack.getSheetAt(0);
>         assertEquals("E11", sheet.getActiveCell().formatAsString());
>         wbBack.close();
>     }

This test is passing on apache-poi 3.15:

    @Test
    public void test61905xls() throws IOException {
        Workbook wb = new HSSFWorkbook();
        Sheet sheet = wb.createSheet("new sheet");
        sheet.setActiveCell(new CellAddress("E11"));
        assertEquals("E11", sheet.getActiveCell().formatAsString());
        wb.write(new FileOutputStream("/tmp/zzz.xls"));
        wb.close();

        Workbook wbBack = new HSSFWorkbook(new FileInputStream("/tmp/zzz.xls"));
        sheet = wbBack.getSheetAt(0);
        assertEquals("E11", sheet.getActiveCell().formatAsString());
        wbBack.close();
    }
Comment 16 Roland Illig 2018-04-25 07:34:12 UTC
(In reply to Davide Angelocola from comment #15)
> This test is passing on apache-poi 3.15:

Sadly, whether this test is passing is not relevant here. POI promises to generate Excel-compatible files, and Excel 2016 does not display the generated file as intended. Therefore POI should be fixed.

https://stackoverflow.com/q/50008212 mentions this bug and suggests a possible workaround, which of course should be provided with a nicer API than using brute-force Java reflection.
Comment 17 Davide Angelocola 2018-04-25 07:51:05 UTC
(In reply to Roland Illig from comment #16)
> (In reply to Davide Angelocola from comment #15)
> > This test is passing on apache-poi 3.15:
> 
> Sadly, whether this test is passing is not relevant here. POI promises to
> generate Excel-compatible files, and Excel 2016 does not display the
> generated file as intended. Therefore POI should be fixed.

I just provided working code for an earlier comment.

> https://stackoverflow.com/q/50008212 mentions this bug and suggests a
> possible workaround, which of course should be provided with a nicer API
> than using brute-force Java reflection.

Thanks!
Comment 18 Dominik Stadler 2018-04-25 14:24:48 UTC
The Stackoverflow discussion shed some light on this, the following seems to make it work, in POI itself we can do this much cleaner, naturally:


    /**
     * Calling just {@code sheet.setActiveCell} has no effect when opening
     * the file with Microsoft Excel 2016.
     */
    private static void setActiveCell(HSSFSheet sheet, CellAddress address) {
        sheet.setActiveCell(address);

        // Following three private fields in a row cannot be the correct path.
        InternalSheet internalSheet = getField(sheet, "_sheet");
        SelectionRecord selection = getField(internalSheet, "_selection");
        CellRangeAddress8Bit[] ranges = getField(selection, "field_6_refs");

        ranges[0].setFirstColumn(address.getColumn());
        ranges[0].setLastColumn(address.getColumn());
        ranges[0].setFirstRow(address.getRow());
        ranges[0].setLastRow(address.getRow());
    }

    private static <T> T getField(Object obj, String fieldName) {
        try {
            Field field = obj.getClass().getDeclaredField(fieldName);
            field.setAccessible(true);
            return (T) field.get(obj);
        } catch (ReflectiveOperationException e) {
            throw new IllegalStateException(e);
        }
    }
Comment 19 Dominik Stadler 2018-04-25 20:16:44 UTC
This should be fixed via r1830115 by populating the field6_refs every time col or row is changed.