From 3e70d22cbc45f1460583127e9c430a461b7852c9 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Mon, 13 Nov 2017 11:46:04 +0000 Subject: [PATCH] Fix setting a font-color if no previous color is defined for the font Add unit-test to verify this git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1815086 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/hssf/util/HSSFColor.java | 2 +- .../apache/poi/xssf/usermodel/XSSFColor.java | 2 +- .../xssf/usermodel/XSSFFontFormatting.java | 2 + .../TestXSSFConditionalFormatting.java | 74 +++++++++++++++++++ .../poi/xssf/usermodel/TestXSSFFont.java | 7 +- 5 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/java/org/apache/poi/hssf/util/HSSFColor.java b/src/java/org/apache/poi/hssf/util/HSSFColor.java index 3c0ba6a9c..51d4af7d6 100644 --- a/src/java/org/apache/poi/hssf/util/HSSFColor.java +++ b/src/java/org/apache/poi/hssf/util/HSSFColor.java @@ -366,7 +366,7 @@ public class HSSFColor implements Color { // Currently the only benefit of this method is to throw an IllegalArgumentException // instead of a ClassCastException. if (color != null && !(color instanceof HSSFColor)) { - throw new IllegalArgumentException("Only HSSFColor objects are supported"); + throw new IllegalArgumentException("Only HSSFColor objects are supported, but had " + color.getClass()); } return (HSSFColor)color; } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java index bb4e422d1..e7dbbb99c 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java @@ -358,7 +358,7 @@ public class XSSFColor extends ExtendedColor { // Currently the only benefit of this method is to throw an IllegalArgumentException // instead of a ClassCastException. if (color != null && !(color instanceof XSSFColor)) { - throw new IllegalArgumentException("Only XSSFColor objects are supported"); + throw new IllegalArgumentException("Only XSSFColor objects are supported, but had " + color.getClass()); } return (XSSFColor)color; } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java index c15d9cdfb..1a18775a4 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFontFormatting.java @@ -121,6 +121,8 @@ public class XSSFFontFormatting implements FontFormatting { XSSFColor xcolor = XSSFColor.toXSSFColor(color); if (xcolor == null) { _font.getColorList().clear(); + } else if(_font.sizeOfColorArray() == 0) { + _font.addNewColor().setRgb(xcolor.getRGB()); } else { _font.setColorArray(0, xcolor.getCTColor()); } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java index 1aa8b3165..8eee77dfc 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFConditionalFormatting.java @@ -25,6 +25,15 @@ import java.io.IOException; import org.apache.poi.ss.usermodel.BaseTestConditionalFormatting; import org.apache.poi.ss.usermodel.Color; +import org.apache.poi.ss.usermodel.ConditionalFormatting; +import org.apache.poi.ss.usermodel.ConditionalFormattingRule; +import org.apache.poi.ss.usermodel.ExtendedColor; +import org.apache.poi.ss.usermodel.FontFormatting; +import org.apache.poi.ss.usermodel.PatternFormatting; +import org.apache.poi.ss.usermodel.Sheet; +import org.apache.poi.ss.usermodel.SheetConditionalFormatting; +import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.xssf.XSSFITestDataProvider; import org.junit.Test; @@ -56,4 +65,69 @@ public class TestXSSFConditionalFormatting extends BaseTestConditionalFormatting public void testReadOffice2007() throws IOException { testReadOffice2007("NewStyleConditionalFormattings.xlsx"); } + + private final static java.awt.Color PEAK_ORANGE = new java.awt.Color(255, 239, 221); + + @Test + public void testFontFormattingColor() { + Workbook wb = XSSFITestDataProvider.instance.createWorkbook(); + final Sheet sheet = wb.createSheet(); + + final SheetConditionalFormatting formatting = sheet.getSheetConditionalFormatting(); + + // the conditional formatting is not automatically added when it is created... + assertEquals(0, formatting.getNumConditionalFormattings()); + ConditionalFormattingRule formattingRule = formatting.createConditionalFormattingRule("A1"); + assertEquals(0, formatting.getNumConditionalFormattings()); + + // adding the formatting makes it available + int idx = formatting.addConditionalFormatting(new CellRangeAddress[] {}, formattingRule); + + // verify that it can be accessed now + assertEquals(0, idx); + assertEquals(1, formatting.getNumConditionalFormattings()); + assertEquals(1, formatting.getConditionalFormattingAt(idx).getNumberOfRules()); + + // this is confusing: the rule is not connected to the sheet, changes are not applied + // so we need to use setRule() explicitly! + FontFormatting fontFmt = formattingRule.createFontFormatting(); + assertNotNull(formattingRule.getFontFormatting()); + assertEquals(1, formatting.getConditionalFormattingAt(idx).getNumberOfRules()); + formatting.getConditionalFormattingAt(idx).setRule(0, formattingRule); + assertNotNull(formatting.getConditionalFormattingAt(idx).getRule(0).getFontFormatting()); + + fontFmt.setFontStyle(true, false); + + assertEquals(-1, fontFmt.getFontColorIndex()); + + //fontFmt.setFontColorIndex((short)11); + final ExtendedColor extendedColor = new XSSFColor(PEAK_ORANGE); + fontFmt.setFontColor(extendedColor); + + PatternFormatting patternFmt = formattingRule.createPatternFormatting(); + assertNotNull(patternFmt); + patternFmt.setFillBackgroundColor(extendedColor); + + assertEquals(1, formatting.getConditionalFormattingAt(0).getNumberOfRules()); + assertNotNull(formatting.getConditionalFormattingAt(0).getRule(0).getFontFormatting()); + assertNotNull(formatting.getConditionalFormattingAt(0).getRule(0).getFontFormatting().getFontColor()); + assertNotNull(formatting.getConditionalFormattingAt(0).getRule(0).getPatternFormatting().getFillBackgroundColorColor()); + + checkFontFormattingColorWriteOutAndReadBack(wb, extendedColor); + } + + private void checkFontFormattingColorWriteOutAndReadBack(Workbook wb, ExtendedColor extendedColor) { + Workbook wbBack = XSSFITestDataProvider.instance.writeOutAndReadBack(wb); + assertNotNull(wbBack); + + assertEquals(1, wbBack.getSheetAt(0).getSheetConditionalFormatting().getNumConditionalFormattings()); + final ConditionalFormatting formattingBack = wbBack.getSheetAt(0).getSheetConditionalFormatting().getConditionalFormattingAt(0); + assertEquals(1, wbBack.getSheetAt(0).getSheetConditionalFormatting().getConditionalFormattingAt(0).getNumberOfRules()); + final ConditionalFormattingRule ruleBack = formattingBack.getRule(0); + final FontFormatting fontFormattingBack = ruleBack.getFontFormatting(); + assertNotNull(formattingBack); + assertNotNull(fontFormattingBack.getFontColor()); + assertEquals(extendedColor, fontFormattingBack.getFontColor()); + assertEquals(extendedColor, ruleBack.getPatternFormatting().getFillBackgroundColorColor()); + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java index 6ab06091a..dd198edf8 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFont.java @@ -80,6 +80,7 @@ public final class TestXSSFFont extends BaseTestFont{ assertEquals(true, ctFont.getBArray(0).getVal()); } + @SuppressWarnings("deprecation") @Test public void testCharSet() throws IOException { CTFont ctFont=CTFont.Factory.newInstance(); @@ -108,7 +109,9 @@ public final class TestXSSFFont extends BaseTestFont{ try { xssfFont.setCharSet(9999); fail("Shouldn't be able to set an invalid charset"); - } catch(POIXMLException e) {} + } catch(POIXMLException e) { + // expected here + } // Now try with a few sample files @@ -120,7 +123,7 @@ public final class TestXSSFFont extends BaseTestFont{ ); wb1.close(); - // GB2312 charact set + // GB2312 charset XSSFWorkbook wb2 = XSSFTestDataSamples.openSampleWorkbook("49273.xlsx"); assertEquals(134, wb2.getSheetAt(0).getRow(0).getCell(0).getCellStyle().getFont().getCharSet()