You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2016/06/18 05:30:05 UTC

[Bug 59719] New: XSSFDataValidationConstraint doesn't parse static list text properly

https://bz.apache.org/bugzilla/show_bug.cgi?id=59719

            Bug ID: 59719
           Summary: XSSFDataValidationConstraint doesn't parse static list
                    text properly
           Product: POI
           Version: 3.15-dev
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: XSSF
          Assignee: dev@poi.apache.org
          Reporter: greg.woolsey@gmail.com

Created attachment 33959
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33959&action=edit
patch to properly parse static list validation constraints

When you add a LIST type validation to a cell in Excel, and type a comma
separated list of values to accept instead of using a range reference or named
range, the value is stored in the XLSX sheet XML as a "formula1" string:

"one, two, three"

named ranges as list validation values are stored in formula1 as

range_name

note the lack of double quotes.

Currently, XSSFDataValidationConstraint just splits all "formula1" strings by
comma.  This makes the first and last values include the double quotes, and all
values contain any whitespace leading or trailing a comma.

Excel automatically trims whitespace when parsing the CSV value strings.

The attached patch checks for and removes the enclosing quotes, and uses a
compiled pattern to split by commas with optional whitespace.

If the formula isn't enclosed in double quotes, it is not parsed into the
explicit values array.

I can't tell from inspecting XLSX files how Excel knows whether a formula is a
list of values or a range, other than the quotes, or falling back on a list if
evaluating as a range fails.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 59719] XSSFDataValidationConstraint doesn't parse static list text properly

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59719

Javen O'Neal <on...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Javen O'Neal <on...@apache.org> ---
It seems like there are more entry points to XSSFDataValidation than necessary,
and each of the entry points end up initializing the member variables in subtly
different ways (leading left quote gets stripped, both quotes get stripped,
neither quote gets stripped; whitespace is removed or not removed from the
formula, etc).

Additionally, even if the storage format overloads formula1, it doesn't mean we
need to overload it.

Since you have experience with this class, could you provide a recommendation
on what constructors or methods should be deprecated to make this class both
easier to maintain and fool-proof for users to use? Save that for a future bug,
I guess.

Applied via r1749265.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 59719] XSSFDataValidationConstraint doesn't parse static list text properly

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59719

Javen O'Neal <on...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #2 from Javen O'Neal <on...@apache.org> ---
I took a stab at writing a unit test in r1749131 given your description in
comment 0. Please let me know if any changes are needed to the unit test, as I
have never used DataValidationConstraints before.

If no changes are needed, let me know and I will close the bug.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 59719] XSSFDataValidationConstraint doesn't parse static list text properly

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59719

--- Comment #1 from Javen O'Neal <on...@apache.org> ---
Applied in r1749129.
Could you write a unit test that shows a scenario where the previous code
failed and the patched code works?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 59719] XSSFDataValidationConstraint doesn't parse static list text properly

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59719

Greg Woolsey <gr...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33959|0                           |1
        is obsolete|                            |

--- Comment #3 from Greg Woolsey <gr...@gmail.com> ---
Created attachment 33964
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33964&action=edit
updates to use constants, add comments, and another unit test case

My latest patch attachment uses the new QUOTE constant one more place and adds
one more unit test, validating the formula1 value is set properly from the
constructor that accepts an array of string literals.

I think with these and your other tests and changes this is ready to go.  Why
the OOXML format overloads formula1 this way is baffling to me.  I bet the old
binary format had some similar form of overloading.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org