[Bug 60845] New: copied cell style and CF

classic Classic list List threaded Threaded
37 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Bug 60845] New: copied cell style and CF

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

            Bug ID: 60845
           Summary: copied cell style and CF
           Product: POI
           Version: 3.15-FINAL
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: XSSF
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ---

Two equal XLSX Files
Both are using conditional Formatting of the Background Color on e.g. Column 1
Now, copy one Row of File1 to File2 using:

for (int i = 0; i < sourceRow.getLastCellNum(); i++) {

    XSSFCellStyle newStyle = destSheet.getWorkbook().createCellStyle();
    newStyle.cloneStyleFrom(oldCell.getCellStyle());      
    newCell.setCellStyle(newStyle);


    // Set the cell data type
    newCell.setCellType(oldCell.getCellType());

    // Set the cell data value
    switch (oldCell.getCellType()) {
    case Cell.CELL_TYPE_BLANK:
        newCell.setCellValue(oldCell.getStringCellValue());
        break;
    case Cell.CELL_TYPE_BOOLEAN:
        newCell.setCellValue(oldCell.getBooleanCellValue());
        break;
    case Cell.CELL_TYPE_ERROR:
        newCell.setCellErrorValue(oldCell.getErrorCellValue());
        break;
    case Cell.CELL_TYPE_FORMULA:
        newCell.setCellFormula(oldCell.getCellFormula());
        break;
    case Cell.CELL_TYPE_NUMERIC:
        newCell.setCellValue(oldCell.getNumericCellValue());
        break;
    case Cell.CELL_TYPE_STRING:
        newCell.setCellValue(oldCell.getRichStringCellValue());
        break;
    }
}

The Row is copied, the values, the size, also the background colors and so on.
But the conditional formatting does not longer work for the
copied rows. "Not working" means: the cells background is white, even if the
value is the right one.

If you use the cell formatting options (in excel) and simply click on "No
Color" for Filling (which is already selected), and confirm it with OK, it
works.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

--- Comment #1 from [hidden email] ---
there is a workaround, if you set the FillPattern by hand via

bla.setFillPattern(FillPatternType.SOLID_FOREGROUND);

for the cells which are in the range of some CF-rule, then the conditional
formatting works. the problem then is, that the cells background becomes BLACK
if the CF-rule(s) do not fire.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #2 from [hidden email] ---
Created attachment 34815
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34815&action=edit
The resulting file, which is not correctly displayed in Excel, but in
LibreOffice

The same problem holds for

Map<String, Object> prop = new HashMap<>();
prop.put(CellUtil.ROTATION, oldCell.getCellStyle().getRotation());
CellUtil.setCellStyleProperties(newCell, prop);

therefore it is not a cloneStyle issue I guess.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #3 from [hidden email] ---
Created attachment 34816
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34816&action=edit
corrupted styles.xml, which causes the described behaviour

i was able to find out which file in the .zip filestructure causes the problem,
it is the styles.xml, i will also add another one - which was created by excel
on the same basis - the latter is working

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #4 from [hidden email] ---
Created attachment 34817
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34817&action=edit
working styles.xml

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #5 from [hidden email] ---
Okay, the problem is the following:

Excel applies conditional formatting only to two sorts of cells:
1) those who have the default fillId=0
2) those who have a patternType like "solid"

But POI creates a new one like:

<patternFill>
<fgColor indexed="64"/>
<bgColor indexed="64"/>
</patternFill>

which does not fit any of those two sorts.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #6 from [hidden email] ---
https://www.java-forums.org/apache-poi/96985-conditional-formatting-does-not-work-copied-cells.html#post417600

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|3.15-FINAL                  |3.16-dev

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #7 from [hidden email] ---
Thanks for all your replies!
I finally found out where the problem is:

1) Excel shows conditional formatting if, and only if, the cell has fillId=0
(the default NONE one) OR if the fillStylePattern != NONE.
Thats a bug in Excel, not in POI.

2) The problem is, that Excel is de facto "Standard".
POI calls like this to set the new Style: setCellStyleProperties >>
setFormatProperties >> setFillPattern

Here is what setFillPattern looks like:

Java Code:
public void setFillPattern(FillPatternType pattern) {
        CTFill ct = getCTFill();
        CTPatternFill ctptrn = ct.isSetPatternFill() ? ct.getPatternFill() :
ct.addNewPatternFill();

        if (pattern == FillPatternType.NO_FILL && ctptrn.isSetPatternType()) {
                ctptrn.unsetPatternType();
        } else {
               
ctptrn.setPatternType(STPatternType.Enum.forInt(pattern.getCode() + 1));
        }

        addFill(ct);
}


It (POI) does simply not check if the FillStyle does already exist, it always
adds a new one. And in general, this is then not fullfilling condtions in 1)

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #8 from [hidden email] ---
Created attachment 34835
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34835&action=edit
a first idea how to patch it

A first approach to fix the issue, works for now but is still in progress. The
idea is to search the styles.xml for already existing fills before adding a new
one with the same properties.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #9 from [hidden email] ---
Okay, I again looked at the code and problem I described at first is caused by
the following thing:

cloneStyleFrom adds the styles from the original to the destination like that:

---

CTFill fill = CTFill.Factory.parse(
    src.getCTFill().toString(), DEFAULT_XML_OPTIONS
);

addFill(fill);

---

I dont't know why, but that results in the following CTFill-String:

<xml-fragment
xmlns:main="http://schemas.openxmlformats.org/spreadsheetml/2006/main"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:x14ac="http://schemas.microsoft.com/office/spreadsheetml/2009/9/ac"
xmlns:x16r2="http://schemas.microsoft.com/office/spreadsheetml/2015/02/main">
  <main:patternFill patternType="none"/>
</xml-fragment>

which is definitely not the same as

<patternFill patternType="none"
xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:x14ac="http://schemas.microsoft.com/office/spreadsheetml/2009/9/ac"
xmlns:x16r2="http://schemas.microsoft.com/office/spreadsheetml/2015/02/main"/>

at least not syntactically.

---

addFill (StylesTable.java) now looks if the new FillStyle is already in the Set
by fills.indexOf(newFill).

indexOf is based on equals of XSSFCellFill, which does a string-Comparison!

---

One way to patch that would be to change the equals function to a semantic
comparison like:

public boolean equals(Object o) {
  if (!(o instanceof XSSFCellFill)) return false;

  XSSFCellFill cf = (XSSFCellFill) o;


  return (
    this.getFillBackgroundColor() == cf.getFillBackgroundColor()
    && this.getFillForegroundColor() == cf.getFillForegroundColor()
    && this.getPatternType() == cf.getPatternType()
  );
}

The downside is the deteriorated performance I guess. And you have to do the
same on XSSFFont.java and XSSFCellBorder.java (and maybe more).

---

Another way is to convert the "fragments" into normal XML inside
cloneStyleFrom(),
I am still looking for a way to do that reliable (using xmlbeans).

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #10 from [hidden email] ---
The XML-Fragment results from the copy() function in CTFill!


 private CTFill getCTFill(){
  CTFill ct;
  // bug 56295: handle missing applyFill attribute as "true" because Excel does
as well
  if(!_cellXf.isSetApplyFill() || _cellXf.getApplyFill()) {
    int fillIndex = (int)_cellXf.getFillId();
    XSSFCellFill cf = _stylesSource.getFillAt(fillIndex);

    ct = (CTFill)cf.getCTFill().copy();
    // ^^^^^^

  } else {
      ct = CTFill.Factory.newInstance();
  }
  return ct;
}

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #11 from [hidden email] ---
Created attachment 34837
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34837&action=edit
patched equals function

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #12 from [hidden email] ---
Created attachment 34838
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34838&action=edit
patched equals function

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

--- Comment #13 from [hidden email] ---
Created attachment 34839
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34839&action=edit
patched equals function

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34835|0                           |1
        is obsolete|                            |
  Attachment #34837|0                           |1
        is obsolete|                            |
  Attachment #34838|0                           |1
        is obsolete|                            |
  Attachment #34839|0                           |1
        is obsolete|                            |

--- Comment #14 from [hidden email] ---
Created attachment 34840
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34840&action=edit
patched equals functions for XSSFFont, XSSFCellFill and XSSFCellBorder

This patch changes the equals functions to a semantic one, rather than the
former syntactic one

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] [PATCH] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|copied cell style and CF    |[PATCH] copied cell style
                   |                            |and CF
           Keywords|                            |PatchAvailable

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] [PATCH] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34840|text/plain                  |application/zip
          mime type|                            |
  Attachment #34840|1                           |0
           is patch|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60845] [PATCH] copied cell style and CF

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60845

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #15 from Dominik Stadler <[hidden email]> ---
Please note that providing a diff instead of the whole source of the files
would be better as the whole files can quickly become outdated due to other
changes to the codebase and thus will make applying the changes more and more
difficult over time.

See http://poi.apache.org/guidelines.html#SubmittingPatches for some automated
ways of creating the patch-files.

Also ideally some additional unit-tests accompany a patch so the changes are
verified and stay in place correctly through future changes to the code.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

12