Bug 59224 - XSSFColor.hasTint() has wrong implementation
Summary: XSSFColor.hasTint() has wrong implementation
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.14-FINAL
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-23 20:54 UTC by gubespam
Modified: 2016-03-24 17:32 UTC (History)
1 user (show)



Attachments
change hasTint, add hasAlpha and unit tests (2.16 KB, patch)
2016-03-24 07:18 UTC, Javen O'Neal
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description gubespam 2016-03-23 20:54:17 UTC
It appears that XSSFColor.hasTint() has an incorrect implementation. It checks to see if the RGB array is length 4 as the determining factor. But other methods in this class indicate that an length of 4 means the color has an *alpha* value.

As a concrete example, try creating a xlsx spreadsheet in Excel, with a color that has a tint value. The hasTint() method will return false, but the getTint() method returns a non-zero value.

So, I think hasTint() should just be renamed to hasAlpha() (and method's javadoc comment updated), and a new method created called hasTint(), with an implemention like below:

    /**
     * A boolean value indicating if the ctColor has alpha or not
     */
    public boolean hasAlpha() {
        if (! ctColor.isSetRgb()) {
            return false;
        }
        return ctColor.getRgb().length == 4;
    }

    /**
     * A boolean value indicating if the ctColor has a tint or not
     */
    public boolean hasTint() {
        return ctColor.getTint() != 0;
    }
Comment 1 Javen O'Neal 2016-03-24 06:43:01 UTC
Should hasTint return false if tint is not set?

    public boolean hasTint() {
        if (!ctColor.isSetTint()) {
            return false;
        }
        return ctColor.getTint() != 0;
    }
Comment 2 Javen O'Neal 2016-03-24 07:18:05 UTC
Created attachment 33700 [details]
change hasTint, add hasAlpha and unit tests

Is this all that needs to be modified?
Comment 3 Javen O'Neal 2016-03-24 17:32:53 UTC
Applied in r1736469.