Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
BalusC opened this issue Feb 11, 2024 · 3 comments
Milestone

Comments

@BalusC
Copy link
Contributor

BalusC commented Feb 11, 2024

Discovered in TCK jakartaee/faces#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
Copy link
Contributor Author

BalusC commented Feb 12, 2024

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.

@BalusC BalusC added this to the 5.0.0 milestone Feb 12, 2024
@chris21k
Copy link

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
Copy link
Contributor

melloware commented Mar 13, 2024

@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);
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants