From 5185ad3b5a22e5dc1161dd00f853c299f128e01a Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Fri, 4 Mar 2011 14:38:13 +0000 Subject: [PATCH] Hopefully fix bug #50846 - Improve how XSSFColor inherits from Themes, by pushing the logic out of XSSFCellStyle and into ThemesTable + make it easier to call git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1077968 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../apache/poi/xssf/model/StylesTable.java | 4 +-- .../apache/poi/xssf/model/ThemesTable.java | 23 +++++++++++++ .../poi/xssf/usermodel/XSSFCellStyle.java | 34 +++++++------------ .../apache/poi/xssf/usermodel/XSSFColor.java | 4 +-- .../usermodel/extensions/XSSFCellBorder.java | 27 ++++++++++----- .../poi/xssf/usermodel/TestXSSFBugs.java | 13 +++++++ .../poi/xssf/usermodel/TestXSSFCellStyle.java | 4 +-- .../poi/xssf/usermodel/TestXSSFColor.java | 14 -------- .../usermodel/extensions/TestXSSFBorder.java | 2 +- 10 files changed, 75 insertions(+), 51 deletions(-) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index b694475025..1ef1f8335e 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 50846 - Improve how XSSFColor inherits from Themes 50847 - XSSFFont now accepts the full range of Charsets from FontChartset 50786 - Speed up calls to HSSFColor.getIndexHash() by returning a cached, unmodifiable Map. HSSFColor.getModifiableIndexHash() provides access to the old (slow but modifiable) functionality 47100 - Change related formulas and named ranges when XSSFWorkbook.setSheetName is called diff --git a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java b/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java index 6edb91c6ae..31844afb8e 100644 --- a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java +++ b/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java @@ -140,7 +140,7 @@ public class StylesTable extends POIXMLDocumentPart { CTBorders ctborders = styleSheet.getBorders(); if(ctborders != null) { for (CTBorder border : ctborders.getBorderArray()) { - borders.add(new XSSFCellBorder(border)); + borders.add(new XSSFCellBorder(border, theme)); } } @@ -433,7 +433,7 @@ public class StylesTable extends POIXMLDocumentPart { fills.add(new XSSFCellFill(ctFill[1])); CTBorder ctBorder = createDefaultBorder(); - borders.add(new XSSFCellBorder(ctBorder)); + borders.add(new XSSFCellBorder(ctBorder, theme)); CTXf styleXf = createDefaultXf(); styleXfs.add(styleXf); diff --git a/src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java b/src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java index 5b8b228691..562837157a 100644 --- a/src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java +++ b/src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java @@ -68,4 +68,27 @@ public class ThemesTable extends POIXMLDocumentPart { } return null; } + + /** + * If the colour is based on a theme, then inherit + * information (currently just colours) from it as + * required. + */ + public void inheritFromThemeAsRequired(XSSFColor color) { + if(color == null) { + // Nothing for us to do + return; + } + if(! color.getCTColor().isSetTheme()) { + // No theme set, nothing to do + return; + } + + // Get the theme colour + XSSFColor themeColor = getThemeColor(color.getTheme()); + // Set the raw colour, not the adjusted one + color.setRgb(themeColor.getCTColor().getRgb()); + + // All done + } } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java index 93b4364d15..55590a2b42 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java @@ -441,8 +441,8 @@ public class XSSFCellStyle implements CellStyle { XSSFCellFill fg = _stylesSource.getFillAt(fillIndex); XSSFColor fillBackgroundColor = fg.getFillBackgroundColor(); - if (fillBackgroundColor != null && fillBackgroundColor.getCTColor().isSetTheme() && _theme != null) { - extractColorFromTheme(fillBackgroundColor); + if (fillBackgroundColor != null && _theme != null) { + _theme.inheritFromThemeAsRequired(fillBackgroundColor); } return fillBackgroundColor; } @@ -477,8 +477,8 @@ public class XSSFCellStyle implements CellStyle { XSSFCellFill fg = _stylesSource.getFillAt(fillIndex); XSSFColor fillForegroundColor = fg.getFillForegroundColor(); - if (fillForegroundColor != null && fillForegroundColor.getCTColor().isSetTheme() && _theme != null) { - extractColorFromTheme(fillForegroundColor); + if (fillForegroundColor != null && _theme != null) { + _theme.inheritFromThemeAsRequired(fillForegroundColor); } return fillForegroundColor; } @@ -766,7 +766,7 @@ public class XSSFCellStyle implements CellStyle { if(border == BORDER_NONE) ct.unsetBottom(); else pr.setStyle(STBorderStyle.Enum.forInt(border + 1)); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -806,7 +806,7 @@ public class XSSFCellStyle implements CellStyle { if(border == BORDER_NONE) ct.unsetLeft(); else pr.setStyle(STBorderStyle.Enum.forInt(border + 1)); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -846,7 +846,7 @@ public class XSSFCellStyle implements CellStyle { if(border == BORDER_NONE) ct.unsetRight(); else pr.setStyle(STBorderStyle.Enum.forInt(border + 1)); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -886,7 +886,7 @@ public class XSSFCellStyle implements CellStyle { if(border == BORDER_NONE) ct.unsetTop(); else pr.setStyle(STBorderStyle.Enum.forInt(border + 1)); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -925,7 +925,7 @@ public class XSSFCellStyle implements CellStyle { if(color != null) pr.setColor(color.getCTColor()); else pr.unsetColor(); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -1194,7 +1194,7 @@ public class XSSFCellStyle implements CellStyle { if(color != null) pr.setColor(color.getCTColor()); else pr.unsetColor(); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -1234,7 +1234,7 @@ public class XSSFCellStyle implements CellStyle { if(color != null) pr.setColor(color.getCTColor()); else pr.unsetColor(); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -1284,7 +1284,7 @@ public class XSSFCellStyle implements CellStyle { if(color != null) pr.setColor(color.getCTColor()); else pr.unsetColor(); - int idx = _stylesSource.putBorder(new XSSFCellBorder(ct)); + int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme)); _cellXf.setBorderId(idx); _cellXf.setApplyBorder(true); @@ -1445,14 +1445,4 @@ public class XSSFCellStyle implements CellStyle { int indexXf = _stylesSource.putCellXf(xf); return new XSSFCellStyle(indexXf-1, xfSize-1, _stylesSource, _theme); } - - /** - * Extracts RGB form theme color. - * @param originalColor Color that refers to theme. - */ - private void extractColorFromTheme(XSSFColor originalColor){ - XSSFColor themeColor = _theme.getThemeColor(originalColor.getTheme()); - // Set the raw colour, not the adjusted one - originalColor.setRgb(themeColor.getCTColor().getRgb()); - } } 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 af5928fbf6..46b2ef2361 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java @@ -16,10 +16,10 @@ ==================================================================== */ package org.apache.poi.xssf.usermodel; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor; import org.apache.poi.hssf.util.HSSFColor; import org.apache.poi.ss.usermodel.Color; import org.apache.poi.util.Internal; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor; /** * Represents a color in SpreadsheetML @@ -329,7 +329,7 @@ public class XSSFColor implements Color { public CTColor getCTColor(){ return ctColor; } - + public int hashCode(){ return ctColor.toString().hashCode(); } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java index 54d584bc6a..adc695b095 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java @@ -18,6 +18,7 @@ package org.apache.poi.xssf.usermodel.extensions; import org.apache.poi.ss.usermodel.BorderStyle; +import org.apache.poi.xssf.model.ThemesTable; import org.apache.poi.xssf.usermodel.XSSFColor; import org.apache.poi.util.Internal; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBorder; @@ -30,22 +31,24 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.STBorderStyle; * Color is optional. */ public class XSSFCellBorder { - + private ThemesTable _theme; private CTBorder border; /** * Creates a Cell Border from the supplied XML definition */ - public XSSFCellBorder(CTBorder border) { + public XSSFCellBorder(CTBorder border, ThemesTable theme) { this.border = border; + this._theme = theme; } /** - * Creates a new, empty Cell Border, on the - * given Styles Table + * Creates a new, empty Cell Border. + * You need to attach this to the Styles Table */ - public XSSFCellBorder() { + public XSSFCellBorder(ThemesTable theme) { border = CTBorder.Factory.newInstance(); + this._theme = theme; } /** @@ -97,8 +100,17 @@ public class XSSFCellBorder { */ public XSSFColor getBorderColor(BorderSide side) { CTBorderPr borderPr = getBorder(side); - return borderPr != null && borderPr.isSetColor() ? - new XSSFColor(borderPr.getColor()) : null; + + if(borderPr != null && borderPr.isSetColor()) { + XSSFColor clr = new XSSFColor(borderPr.getColor()); + if(_theme != null) { + _theme.inheritFromThemeAsRequired(clr); + } + return clr; + } else { + // No border set + return null; + } } /** @@ -155,5 +167,4 @@ public class XSSFCellBorder { XSSFCellBorder cf = (XSSFCellBorder) o; return border.toString().equals(cf.getCTBorder().toString()); } - } \ No newline at end of file diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index 3b17b9ac0d..465d164c26 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -725,6 +725,19 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { assertEquals("FFCCFFCC", cs.getFillForegroundColorColor().getARGBHex()); } + /** + * If the border colours are set with themes, then we + * should still be able to get colours + */ + public void test50846() throws Exception { + // TODO Get file and test + //Workbook wb = XSSFTestDataSamples.openSampleWorkbook("50846.xlsx"); + + // Check the style that is theme based + + // Check the one that isn't + } + /** * Fonts where their colours come from the theme rather * then being set explicitly still should allow the diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java index 02b34ae85e..83edc8f2f4 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java @@ -59,11 +59,11 @@ public class TestXSSFCellStyle extends TestCase { ctStylesheet = stylesTable.getCTStylesheet(); ctBorderA = CTBorder.Factory.newInstance(); - XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA); + XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA, null); long borderId = stylesTable.putBorder(borderA); assertEquals(1, borderId); - XSSFCellBorder borderB = new XSSFCellBorder(); + XSSFCellBorder borderB = new XSSFCellBorder(null); assertEquals(1, stylesTable.putBorder(borderB)); ctFill = CTFill.Factory.newInstance(); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java index 17b1d0d547..60dfb4f97d 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java @@ -19,21 +19,7 @@ package org.apache.poi.xssf.usermodel; import junit.framework.TestCase; -import org.apache.poi.ss.usermodel.*; -import org.apache.poi.xssf.XSSFITestDataProvider; import org.apache.poi.xssf.XSSFTestDataSamples; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBooleanProperty; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFont; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFontName; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFontScheme; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFontSize; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTIntProperty; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTUnderlineProperty; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTVerticalAlignFontProperty; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STFontScheme; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STUnderlineValues; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STVerticalAlignRun; public final class TestXSSFColor extends TestCase { public void testIndexedColour() throws Exception { diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java index e7233c227e..46af27d59d 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java @@ -40,7 +40,7 @@ public class TestXSSFBorder extends TestCase { right.setStyle(STBorderStyle.NONE); bottom.setStyle(STBorderStyle.THIN); - XSSFCellBorder cellBorderStyle = new XSSFCellBorder(border); + XSSFCellBorder cellBorderStyle = new XSSFCellBorder(border, null); assertEquals("DASH_DOT", cellBorderStyle.getBorderStyle(BorderSide.TOP).toString()); assertEquals("NONE", cellBorderStyle.getBorderStyle(BorderSide.RIGHT).toString()); -- 2.39.5