Bug 51222

There seems to be an issue with how we look up colours in the themes table. I've added a unit test in r1125559 that shows the issue. A4 references theme 2, A5 references theme 5, and in the themes table the colours look to be the wrong way around.

More investigation is needed to figure out why the themes appear to be the wrong way round in the file

Having read the spec again, I'm with POI on this one. The file really does seem to be requesting the colours in the wrong order!

There's presumably something somewhere the inverts a meaning of something, or swaps something round, but I've no idea what or where, and everything I've read says we're doing the right thing...

I've also had this problem.  3.8 beta 4 seems to invert black and white.

I tested with 3.8 beta 4, which shows the problem, and 3.7, which is correct.

@Test
	public void testXSSFColor(){
		//Ver 3.8beta4 of poi-xml seems buggy
		Color c = new Color(0,0,0);
		XSSFColor c2 = new XSSFColor(c);
		
		//Actually, ver 3.7 seems wrong too - Shouldn't ARGB black be FF000000?
		//3.8 beta 4 returns FFFFFFFF
		//3.7 returns 000000
		assertTrue(c2.getARGBHex().equals("000000"));
		
		Color c3 = new Color(255,255,255);
		XSSFColor c4 = new XSSFColor(c3);
		//3.8 beta 4 returns FF000000
		//3.7 returns FFFFFF
		assertTrue(c4.getARGBHex().equals("FFFFFF"));
	}

Net result is that if you specify new new XSSFColor(Color(0,0,0)) as the font colour, you get white text.

Created attachment 29832 [details]
Themes.xlsx - Shows colors in theme number order

Created attachment 29834 [details]
patch.tar.gz – Changed files, changed test cases, new files

I believe I have the explanation for why these specific colors get reversed.  This also explains why black and white get reversed in Bugs 51236, 52079, and 53274.

Whenever certain Theme colors are used, the following colors get swapped in POI:
- FFFFFF and 000000 (white and black) (themes 0 and 1, respectively)
- EEECE1 and 1F497D (tan and dark blue) (themes 2 and 3, respectively)

Here is an excerpt from a typical “theme1.xml” file from an .xlsx file:

<a:clrScheme name="Office">
  <a:dk1><a:sysClr val="windowText" lastClr="000000"/></a:dk1>
  <a:lt1><a:sysClr val="window" lastClr="FFFFFF"/></a:lt1>
  <a:dk2><a:srgbClr val="1F497D"/></a:dk2>
  <a:lt2><a:srgbClr val="EEECE1"/></a:lt2>
  <a:accent1><a:srgbClr val="4F81BD"/></a:accent1>
  <a:accent2><a:srgbClr val="C0504D"/></a:accent2>
  <a:accent3><a:srgbClr val="9BBB59"/></a:accent3>
  <a:accent4><a:srgbClr val="8064A2"/></a:accent4>
  <a:accent5><a:srgbClr val="4BACC6"/></a:accent5>
  <a:accent6><a:srgbClr val="F79646"/></a:accent6>
  <a:hlink><a:srgbClr val="0000FF"/></a:hlink>
  <a:folHlink><a:srgbClr val="800080"/></a:folHlink>
</a:clrScheme>

However, theme 0 is NOT black, it’s white, and theme 1 is NOT white, it’s black.  Additionally, theme 2 is NOT dark blue, it’s tan, and theme 3 is NOT tan, it’s dark blue.  (The other theme colors are in order.) 
I’ve attached a spreadsheet “Themes.xlsx” that contains one cell per row, with text cells colored in theme color order (0-11).

Changing the order of the color elements in the color scheme in theme1.xml does not affect the colors of the text in the resultant spreadsheet.  Therefore, Excel is not taking the theme number as an array index into the list of theme colors; it must be mapping the theme number to specific theme color elements:
- 0 => a:lt1 (LISTED SECOND in theme1.xml!)
- 1 => a:dk1 (LISTED FIRST!)
- 2 => a:lt2 (LISTED FOURTH!)
- 3 => a:dk2 (LISTED THIRD!)
- 4 => a:accent1
- 5 => a:accent2
- 6 => a:accent3
- 7 => a:accent4
- 8 => a:accent5
- 9 => a:accent6
- 10 => a:hlink
- 11 => a:folHlink

POI versions 3.8 and 3.9 applied a fix to this issue in XSSFColor.java and ThemesTable.java that is incorrect, namely, using the “correctRGB” method in XSSFColor.java, which flips black and white.  But the fix itself is incorrect.  It did not resolve the issue or explain why black and white get reversed.

I have attached a patch that makes the following changes to resolve the above issues:
- src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java: Removed the flawed “correctRGB” method, and removed all calls to it.  (It was private, so all calls to it were only in XSSFColor.java.)
- src/ooxml/java/org/apache/poi/xssf/model/ThemesTables.java: Introduced a Map of theme numbers to XSSFColors.  Created a new private method “cacheThemeColors” that is called by the constructors to initialize the Map.  Moved the logic that extracts the XSSFColor from the theme color element to a new private method “getXSSFColor”.  The existing method “getThemeColor” now accesses the Map to retrieve an XSSFColor.
- src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java: Changed a few incorrect “assertEquals” statements.  When tinting white darker, the black/white flip no longer occurs, so white (255, 255, 255) gets tinted darker to (165, 165, 165), not to (0, 0, 0); the byte array is really [-91, -91, -91].  Later, when assigning the color black, now the correct byte array [0, 0, 0] is passed in instead of [-1, -1, -1], and the ARGB string is expected to be “FF000000” instead of “FFFFFFFF”.
- src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFCellFill.java: This attempted to verify a tinted theme color, dark blue (dk2), but the RGB codes in the test were for tan (lt2).  These values are now corrected.
- src/ooxml/testcases/org/apache/poi/xssf/model/TestThemesTable.java: A new JUnit test for ThemesTable.java.
- test-data/spreadsheet/Themes.xlsx: This new spreadsheet is the same as the one mentioned above.  It helps test ThemesTable.java in TestThemesTable.java.

I guess I needed to set the status of this bug too.

This doesn't appear to have been fixed yet, merely has a patch available, so re-opening and tagging as such

Is there a target release for incorporating this patch?

It would be good if someone could try the patch, and confirm:
 * Does it apply cleanly?
 * Do all existing unit tests pass?
 * Do new unit tests pass?
 * Do new unit tests cover all of the changed functionality, and all cases?
 * Does Excel then show the right thing?

If any of those are a no, then more work is needed before it can be applied

Applied with some modifications in r1621393

> * Does it apply cleanly?
> * Do all existing unit tests pass?
> * Do new unit tests pass?
Jenkins will tell us ;)

> * Do new unit tests cover all of the changed functionality, and all cases?
There are 12 (indexed) theme attributes, which are tested -> yes

> * Does Excel then show the right thing?
The TestThemesTable test can be used to generate themed and fixed color cells side-by-side, which looks good.

*** Bug 52079 has been marked as a duplicate of this bug. ***

*** Bug 51236 has been marked as a duplicate of this bug. ***