ASF Bugzilla – Attachment 33254 Details for
Bug 51622
autoSizeColumn incorrectly sizes columns containing leading whitespace
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
SheetUtil.java fix and corresponding unit test
bug51622.patch (text/plain), 6.68 KB, created by
Javen O'Neal
on 2015-11-04 06:38:58 UTC
(
hide
)
Description:
SheetUtil.java fix and corresponding unit test
Filename:
MIME Type:
Creator:
Javen O'Neal
Created:
2015-11-04 06:38:58 UTC
Size:
6.68 KB
patch
obsolete
>Index: src/java/org/apache/poi/ss/util/SheetUtil.java >=================================================================== >--- src/java/org/apache/poi/ss/util/SheetUtil.java (revision 1712472) >+++ src/java/org/apache/poi/ss/util/SheetUtil.java (working copy) >@@ -21,6 +21,7 @@ > import java.awt.font.TextAttribute; > import java.awt.font.TextLayout; > import java.awt.geom.AffineTransform; >+import java.awt.geom.Rectangle2D; > import java.text.AttributedString; > import java.util.Locale; > import java.util.Map; >@@ -165,6 +166,7 @@ > private static double getCellWidth(int defaultCharWidth, int colspan, > CellStyle style, double width, AttributedString str) { > TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext); >+ final Rectangle2D bounds; > if(style.getRotation() != 0){ > /* > * Transform the text using a scale so that it's height is increased by a multiple of the leading, >@@ -177,10 +179,13 @@ > trans.concatenate( > AffineTransform.getScaleInstance(1, fontHeightMultiple) > ); >- width = Math.max(width, ((layout.getOutline(trans).getBounds().getWidth() / colspan) / defaultCharWidth) + style.getIndention()); >+ bounds = layout.getOutline(trans).getBounds(); > } else { >- width = Math.max(width, ((layout.getBounds().getWidth() / colspan) / defaultCharWidth) + style.getIndention()); >+ bounds = layout.getBounds(); > } >+ // entireWidth accounts for leading spaces which is excluded from bounds.getWidth() >+ final double frameWidth = bounds.getX() + bounds.getWidth(); >+ width = Math.max(width, ((frameWidth / colspan) / defaultCharWidth) + style.getIndention()); > return width; > } > >@@ -300,6 +305,7 @@ > } > > public static boolean containsCell(CellRangeAddress cr, int rowIx, int colIx) { >+ //FIXME: isn't this the same as cr.isInRange(rowInd, colInd) ? > if (cr.getFirstRow() <= rowIx && cr.getLastRow() >= rowIx > && cr.getFirstColumn() <= colIx && cr.getLastColumn() >= colIx) > { >Index: src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java >=================================================================== >--- src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java (revision 1712472) >+++ src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java (working copy) >@@ -31,6 +31,8 @@ > import java.util.HashMap; > import java.util.Map; > >+import java.awt.geom.Rectangle2D; >+ > import org.apache.poi.hssf.usermodel.HSSFWorkbook; > import org.apache.poi.hssf.util.PaneInformation; > import org.apache.poi.ss.ITestDataProvider; >@@ -416,7 +418,64 @@ > sheet.setColumnWidth(0, sheet.getColumnWidth(0)); // Bug 50681 reports exception at this point > wb.close(); > } >+ >+ @Test >+ public final void bug51622_testAutoSizeShouldRecognizeLeadingSpaces() throws IOException { >+ Workbook wb = _testDataProvider.createWorkbook(); >+ BaseTestSheetAutosizeColumn.fixFonts(wb); >+ Sheet sheet = wb.createSheet(); >+ Row row = sheet.createRow(0); >+ Cell cell0 = row.createCell(0); >+ Cell cell1 = row.createCell(1); >+ Cell cell2 = row.createCell(2); >+ >+ cell0.setCellValue("Test Column AutoSize"); >+ cell1.setCellValue(" Test Column AutoSize"); >+ cell2.setCellValue("Test Column AutoSize "); >+ >+ sheet.autoSizeColumn(0); >+ sheet.autoSizeColumn(1); >+ sheet.autoSizeColumn(2); >+ >+ int noWhitespaceColWidth = sheet.getColumnWidth(0); >+ int leadingWhitespaceColWidth = sheet.getColumnWidth(1); >+ int trailingWhitespaceColWidth = sheet.getColumnWidth(2); >+ >+ // Based on the amount of text and whitespace used, and the default font >+ // assume that the cell with whitespace should be at least 20% wider than >+ // the cell without whitespace. This number is arbitrary, but should be large >+ // enough to guarantee that the whitespace cell isn't wider due to chance. >+ // Experimentally, I calculated the ratio as 1.2478181, though this ratio may change >+ // if the default font or margins change. >+ final double expectedRatioThreshold = 1.2f; >+ double leadingWhitespaceRatio = ((double) leadingWhitespaceColWidth)/noWhitespaceColWidth; >+ double trailingWhitespaceRatio = ((double) leadingWhitespaceColWidth)/noWhitespaceColWidth; >+ >+ assertGreaterThan("leading whitespace is longer than no whitespace", leadingWhitespaceRatio, expectedRatioThreshold); >+ assertGreaterThan("trailing whitespace is longer than no whitespace", trailingWhitespaceRatio, expectedRatioThreshold); >+ assertEquals("cells with equal leading and trailing whitespace have equal width", >+ leadingWhitespaceColWidth, trailingWhitespaceColWidth); >+ >+ wb.close(); >+ } >+ >+ /** >+ * Test if a > b. Fails if false. >+ * >+ * @param message >+ * @param a >+ * @param b >+ */ >+ private void assertGreaterThan(String message, double a, double b) { >+ if (a > b) { // expected >+ } else { >+ String msg = "Expected: " + a + " > " + b; >+ fail(message + ": " + msg); >+ } >+ } > >+ // FIXME: this function is a self-fulfilling prophecy: this test will always pass as long >+ // as the code-under-test and the testcase code are written the same way (have the same bugs). > private double computeCellWidthManually(Cell cell0, Font font) { > final FontRenderContext fontRenderContext = new FontRenderContext(null, true, true); > RichTextString rt = cell0.getRichStringCellValue(); >@@ -432,8 +491,15 @@ > } > > TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext); >- return ((layout.getBounds().getWidth() / 1) / 8); >+ double frameWidth = getFrameWidth(layout); >+ return ((frameWidth / 1) / 8); > } >+ >+ private double getFrameWidth(TextLayout layout) { >+ Rectangle2D bounds = layout.getBounds(); >+ double frameWidth = bounds.getX() + bounds.getWidth(); >+ return frameWidth; >+ } > > private double computeCellWidthFixed(Font font, String txt) { > final FontRenderContext fontRenderContext = new FontRenderContext(null, true, true); >@@ -441,7 +507,8 @@ > copyAttributes(font, str, 0, txt.length()); > > TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext); >- return layout.getBounds().getWidth(); >+ double frameWidth = getFrameWidth(layout); >+ return frameWidth; > } > > private static void copyAttributes(Font font, AttributedString str, int startIdx, int endIdx) {
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 51622
:
27352
|
27353
|
33253
| 33254