On Tue, 10 Feb 2026 15:09:55 GMT, Michael Strauß <[email protected]> wrote:
>> Color.web(string, double) parses a color string by creating substrings of
>> the input. These string allocations can be removed.
>>
>> There are no new tests for the `Color` class, since the existing tests
>> already cover all relevant code paths.
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> CSS-compliant double parser
The JavaFX CSS spec states (under <number>):
Real numbers and integers are specified in decimal notation only. ... A
<number> can either be an <integer>, or it can be zero or more digits followed
by a dot (.) followed by one or more digits. Both integers and real numbers may
be preceded by a "-" or "+" to indicate the sign. -0 is equivalent to 0 and is
not a negative number.
[+|-]? [[0-9]+|[0-9]*"."[0-9]+]
it looks to me like exponential notation should not be supported.
Mandatory disclaimer: W3C is not a real standard.
I am not sure if we want to change the CSS spec to allow scientific notation in
all the <number>s, I don't even sure we want to add this support in
`Color.web()`. If we do though, I think we would need to update the javadoc
and possibly the CSS Reference, right?
I must admit I don't know how much changes in `Color.web()` impact CSS parsing.
For example, it looks like `Color.web()` is being called from
`CssParser.colorValueOfString(String)` but a simple css like
`-fx-background-color: rgb(1);` won't trigger that code.
it's not a real standard in a sense that it is an attempt to write a spec for
something that every browser manufacturer want to change to make their browser
"better". maybe the situation changed recently, I don't know.
But I do agree with you - the majority of developers are familiar with it, so
it might make sense to adapt in some cases (while keeping in mind possible
regressions).
> JavaFX contradicts its own specification, but the specification is (and has
> always been) wrong anyway.
This is hilarious and concerning at the same time. I wonder whether we need to
update the javadoc / create a CSR since we are changing the behavior. What do
you think?
modules/javafx.graphics/src/main/java/javafx/scene/paint/Color.java line 437:
> 435:
> 436: if (len == 3) {
> 437: r = Integer.parseInt(colorString, offset, offset + 1,
> 16);
would a switch statement be better in this case (e.g. case when len == 1)?
modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java
line 40:
> 38: @Test
> 39: public void parseIntegerAndSignedInteger() {
> 40: assertEquals(0.0, parseDouble("0"), 0.0);
The JavaFX CSS spec also states
-0 is equivalent to 0 and is not a negative number.
How should we test this?
modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java
line 79:
> 77: assertNumberFormatException("1 ");
> 78: assertNumberFormatException("xx1e2yy", 2, 6); // slice is "1e2y"
> 79: }
maybe add another test for malformed numbers such as `++0`, `1e--2`, etc.?
modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java
line 270:
> 268: return rawBits >= 0 ? rawBits : (0x8000_0000_0000_0000L -
> rawBits);
> 269: }
> 270: }
missing newline
-------------
PR Review: https://git.openjdk.org/jfx/pull/2069#pullrequestreview-3780071461
PR Comment: https://git.openjdk.org/jfx/pull/2069#issuecomment-3880094916
PR Comment: https://git.openjdk.org/jfx/pull/2069#issuecomment-3880191468
PR Comment: https://git.openjdk.org/jfx/pull/2069#issuecomment-3880669134
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2789907975
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2789129871
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2789098613
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2788929245
