search

eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
158 stars 107 forks source link

JDK21 default DateTimeFormatter behind <f:convertDateTime type="localTime" /> doesn't anymore accept plain vanilla space #5399

Open BalusC opened 4 months ago

BalusC commented 4 months ago

Discovered in TCK https://github.com/jakartaee/faces/pull/1882

The following doesn't anymore work for me with OpenJDK 21.0.1:

<h:inputText id="localTime" value="#{backingBean.localTime}">
    <f:convertDateTime type="localTime" />
</h:inputText>

Assuming en_US locale, and no pattern specified, this creates under the covers a default formatter as follows:

DateTimeFormatter.ofLocalizedTime(FormatStyle.MEDIUM).withLocale(Locale.US);

which essentially defaults to a pattern h:mm:ss a however the whitespace before the a has somehow become a NNBSP \u202f instead of a plain vanilla space \u0020. This means that the enduser has to literally type and submit the NNBSP character in order to get it to convert successfully. This makes no sense. I'm not terribly sure if the bug is in JDK or in Faces/Mojarra. But this is definitely something to look at. There should be a way to extract the pattern from the formatter and then replace all spurious whitespace by a normal space and then recreate the formatter based on pattern instead of FormatStyle as that appears to be intended as output-only somehow.

BalusC commented 4 months ago

On second thought, I believe defaulting to an arbitrary FormatStyle for parsing was a bad idea from the beginning on because this is locale specific nonetheless. The pattern of <f:convertDateTime type="..."> should in case of converting a string to java.time type have defaulted to an ISO format, as defined in among others LocalTime#parse(String) (so the parse() method without formatter argument).

We should probably revise the spec on this for 5.0.

On the other hand it's also possible that the original test was bad, that it shouldn't have relied on a locale/JDK specific output pattern, but that it should have used a predefined parsing pattern. The goal of these tests was nonetheless whether the input is conversible to a java.time type in bean.

chris21k commented 3 months ago

Please see here for more information: https://bugs.openjdk.org/browse/JDK-8324308 https://unicode-org.atlassian.net/browse/CLDR-17324

As far as I can understand, Unicode specification should allow different types of spaces.

Chris

melloware commented 3 months ago

@BalusC yes I got burned by this somewhere between JDK 11 and 17 when the Brazilian currency when from SPACE to NBSP.

I had to add this code to the CurrencyValidator in PrimeFaces...

https://github.com/primefaces/primefaces/blob/b96b824f895030c4f1b702c5e4918d37ac15f14b/primefaces/src/main/java/org/primefaces/util/CurrencyValidator.java#L142-L145

        // between JDK8 and 11 some space characters became non breaking space '\u00A0'
        if (formatter.getPositivePrefix().indexOf(Constants.NON_BREAKING_SPACE) >= 0) {
            value = value.replaceAll(Constants.SPACE, Constants.NON_BREAKING_SPACE_STR);
        }