Fixing the offset calculation and passing the top-left-most region rather than the region matching the current cell fixed these cases. I've augmented unit tests to check for them to avoid future regressions. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1855619 13f79535-47bb-0310-9956-ffa450edef68pull/143/head
@@ -69,6 +69,9 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon | |||
/* cached values */ | |||
private final CellRangeAddress[] regions; | |||
private CellRangeAddress topLeftRegion; | |||
/** | |||
* Depending on the rule type, it may want to know about certain values in the region when evaluating {@link #matches(CellReference)}, | |||
* such as top 10, unique, duplicate, average, etc. This collection stores those if needed so they are not repeatedly calculated | |||
@@ -114,6 +117,14 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon | |||
this.priority = rule.getPriority(); | |||
this.regions = regions; | |||
for (CellRangeAddress region : regions) { | |||
if (topLeftRegion == null) topLeftRegion = region; | |||
else if (region.getFirstColumn() < topLeftRegion.getFirstColumn() | |||
|| region.getFirstRow() < topLeftRegion.getFirstRow()) { | |||
topLeftRegion = region; | |||
} | |||
} | |||
formula1 = rule.getFormula1(); | |||
formula2 = rule.getFormula2(); | |||
@@ -316,13 +327,13 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon | |||
if (ruleType.equals(ConditionType.CELL_VALUE_IS)) { | |||
// undefined cells never match a VALUE_IS condition | |||
if (cell == null) return false; | |||
return checkValue(cell, region); | |||
return checkValue(cell, topLeftRegion); | |||
} | |||
if (ruleType.equals(ConditionType.FORMULA)) { | |||
return checkFormula(ref, region); | |||
return checkFormula(ref, topLeftRegion); | |||
} | |||
if (ruleType.equals(ConditionType.FILTER)) { | |||
return checkFilter(cell, ref, region); | |||
return checkFilter(cell, ref, topLeftRegion); | |||
} | |||
// TODO: anything else, we don't handle yet, such as top 10 |
@@ -880,20 +880,16 @@ public final class WorkbookEvaluator { | |||
* <p><pre>formula ref + current cell ref</pre></p> | |||
* @param ptgs | |||
* @param target cell within the region to use. | |||
* @param region containing the cell | |||
* @param region containing the cell, OR, for conditional format rules with multiple ranges, the region with the top-left-most cell | |||
* @return true if any Ptg references were shifted | |||
* @throws IndexOutOfBoundsException if the resulting shifted row/column indexes are over the document format limits | |||
* @throws IllegalArgumentException if target is not within region. | |||
*/ | |||
protected boolean adjustRegionRelativeReference(Ptg[] ptgs, CellReference target, CellRangeAddressBase region) { | |||
if (! region.isInRange(target)) { | |||
throw new IllegalArgumentException(target + " is not within " + region); | |||
} | |||
//return adjustRegionRelativeReference(ptgs, target.getRow() - region.getFirstRow(), target.getCol() - region.getFirstColumn()); | |||
// region may not be the one that contains the target, if a conditional formatting rule applies to multiple regions | |||
int deltaRow = target.getRow(); | |||
int deltaColumn = target.getCol(); | |||
int deltaRow = target.getRow() - region.getFirstRow(); | |||
int deltaColumn = target.getCol() - region.getFirstColumn(); | |||
boolean shifted = false; | |||
for (Ptg ptg : ptgs) { | |||
@@ -902,7 +898,7 @@ public final class WorkbookEvaluator { | |||
RefPtgBase ref = (RefPtgBase) ptg; | |||
// re-calculate cell references | |||
final SpreadsheetVersion version = _workbook.getSpreadsheetVersion(); | |||
if (ref.isRowRelative()) { | |||
if (ref.isRowRelative() && deltaRow > 0) { | |||
final int rowIndex = ref.getRow() + deltaRow; | |||
if (rowIndex > version.getMaxRows()) { | |||
throw new IndexOutOfBoundsException(version.name() + " files can only have " + version.getMaxRows() + " rows, but row " + rowIndex + " was requested."); | |||
@@ -910,7 +906,7 @@ public final class WorkbookEvaluator { | |||
ref.setRow(rowIndex); | |||
shifted = true; | |||
} | |||
if (ref.isColRelative()) { | |||
if (ref.isColRelative() && deltaColumn > 0) { | |||
final int colIndex = ref.getColumn() + deltaColumn; | |||
if (colIndex > version.getMaxColumns()) { | |||
throw new IndexOutOfBoundsException(version.name() + " files can only have " + version.getMaxColumns() + " columns, but column " + colIndex + " was requested."); |
@@ -19,6 +19,7 @@ package org.apache.poi.ss.usermodel; | |||
import java.io.IOException; | |||
import java.util.Date; | |||
import java.util.List; | |||
import org.apache.poi.ss.formula.ConditionalFormattingEvaluator; | |||
@@ -117,6 +118,28 @@ public class ConditionalFormattingEvalTest { | |||
getRulesFor(8,2); | |||
assertEquals("wrong # of rules for " + ref, 1, rules.size()); | |||
sheet = wb.getSheet("Compare to totals"); | |||
getRulesFor(3, 2); | |||
assertEquals("wrong # of rules for " + ref, 1, rules.size()); | |||
assertEquals("wrong fg color for " + ref, "FFFF0000", getColor(rules.get(0).getRule().getFontFormatting().getFontColor())); | |||
getRulesFor(3, 3); | |||
assertEquals("wrong # of rules for " + ref, 0, rules.size()); | |||
getRulesFor(15, 4); | |||
assertEquals("wrong # of rules for " + ref, 0, rules.size()); | |||
getRulesFor(16, 1); | |||
assertEquals("wrong # of rules for " + ref, 1, rules.size()); | |||
assertEquals("wrong fg color for " + ref, "FFFF0000", getColor(rules.get(0).getRule().getFontFormatting().getFontColor())); | |||
sheet = wb.getSheet("Products3"); | |||
sheet.getRow(8).getCell(0).setCellValue(new Date()); | |||
getRulesFor(8, 0); | |||
assertEquals("wrong # of rules for " + ref, 1, rules.size()); | |||
getRulesFor(8, 3); | |||
assertEquals("wrong # of rules for " + ref, 1, rules.size()); | |||
sheet = wb.getSheet("Customers2"); | |||
getRulesFor(3, 0); | |||
assertEquals("wrong # of rules for " + ref, 0, rules.size()); | |||
} | |||
@Test |