[OpenJDK 2D-Dev] 5082756: Image I/O plug-ins set metadata boolean attributes to "true" or "false"
Martin von Gagern
Martin.vGagern at gmx.net
Mon Nov 10 10:58:51 UTC 2008
Bug ID: 5082756; State: 6-Fix Understood, bug; Priority: 4-Low
While the DTDs for the different metdata formats usually specify boolean
values as ( "TRUE" | "FALSE" ), the implementations tend to use "true"
and "false" instead.
There are two possible approaches to this problem:
1. Have the implementation match the specification, i.e. use upper case
2. Adjust the specification to match implementation, i.e. use lower case
While the former has the benefit of only touching internal com.sun
implementation classes, the latter has the long therm benefit that
implementations can use the default String conversion methods from class
Boolean without further case conversion.
While I would have wished for the specification to use lower case in the
first place, I would now stick with the way it is, and adjust the
implementation to upper case.
Both approaches can be implemented with different degrees of tolerance
when accepting values:
a) strict: only allow the values also allowed by the DTD
b) two possibilities: allow both "true" and "TRUE"
c) ignore case: also allow "tRuE"
Here I am in favor of the middle way. This gives you backward
compatibility, but won't allow additional values without reason. The
benefit is that with this it is more likely that an application
developed under OpenJDK will work under other JREs as well. The downside
of course is that applications developed for a JRE that completely
ignores case would fail on OpenJDK. As I expect OpenJDK to have a larger
market share, I would think this less likely to get unnoticed, though.
There is also a slight performance benefit from not allowing mixed case,
but I guess thats negligible here.
So corresponding to my personal preferences, the attached patch changes
all attribute creations to uppercase, and allows for both cases in
PNGMetadata, but no mixed forms. The GIFMetadata implementation which
already uses equalsIgnoreCase and thus allows for mixed case I left
untouched for the moment. For the sake of consistency, we might to
change that to the more strict two cases instead.
I haven't any test case ready yet. I'll write one when we are agreed on
the intended behaviour.
The attached patch is from my mercurial patch queue. Once you consider
it ready for inclusion, I will commit it locally and export a mercurial
Martin von Gagern
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
More information about the 2d-dev