pjfanning opened a new pull request, #1338: URL: https://github.com/apache/poi/pull/1338
Fixes https://bz.apache.org/bugzilla/show_bug.cgi?id=69366 ### Problem `CellUtil.setCellStyleProperties` looks for an existing style whose properties equal the wanted ones before creating a new one. The fill colours are held twice in those properties — as a `Color` object and as an indexed short — and `styleMapsMatch` required both to be equal. That fails in two ways, each of which creates a new cell style on every call (the reporter's loop ends up with 1002 styles instead of 2): 1. `FILL_FOREGROUND_COLOR → null` (or `FILL_FOREGROUND_COLOR_COLOR → null`): the wanted map holds `null` (or no key), but every existing style without a fill colour reports `64` (AUTOMATIC) from `getFillForegroundColor()`. 2. `FILL_FOREGROUND_COLOR_COLOR → an RGB XSSFColor`: the style created for the first cell reads back an indexed value of `0` (`XSSFColor.getIndexed()` of a non-indexed colour), while the next fresh cell's wanted map still carries `64` — the `Color` objects are equal but the meaningless shorts differ. This is why the reporter's "same cell" RGB test passed and the "different cell" one failed. ### Fix `styleMapsMatch` now compares the two fill colours through a small helper: a wanted `Color` is authoritative and the derived indexed values are ignored; only when there is no wanted `Color` are the indexed values compared, with a missing/null value meaning the automatic colour. The `disableNullColorCheck` contract of `setCellStyleProperty(cell, *_COLOR_COLOR, null)` (a cleared colour must not match a style that still has one) is preserved. HSSF was affected by the null cases as well (its `getFillForegroundColor()` also reports 64) and is fixed by the same change; `HSSFColor.equals` includes the index, so comparing `Color`s alone loses nothing there. ### Tests - `BaseTestCellUtil` (runs for HSSF, XSSF and SXSSF): null indexed colours and null `Color`s reuse the style across calls and cells; indexed RED vs BLUE are still told apart. - `TestXSSFCellUtil`: the RGB `Color` case across calls and cells, and a different RGB colour still gets its own style. - Without the fix, the null tests fail on HSSF/XSSF/SXSSF and the RGB test fails on XSSF. Note: this touches the same `CellUtil` method region's neighbourhood as #1337 (bug 69463) but a different function; both add tests at the end of `TestXSSFCellUtil`, so whichever merges second will need a trivial conflict resolution there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
