On Mon, 15 Dec 2025 22:57:24 GMT, Andy Goryachev <[email protected]> wrote:

>> Michael Strauß has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 14 commits:
>> 
>>  - Merge branch 'master' into feature/media-features-viewport-characteristics
>>  - resolve merge conflicts
>>  - Merge branch 'master' into feature/media-features-viewport-characteristics
>>    
>>    # Conflicts:
>>    # 
>> modules/javafx.graphics/src/test/java/test/javafx/css/CssParser_mediaQuery_Test.java
>>  - update cssref.html
>>  - Merge branch 'master' into feature/media-features-viewport-characteristics
>>  - whitespace, add final modifier
>>  - Merge branch 'master' into feature/media-features-viewport-characteristics
>>  - Refactor context awareness
>>  - Merge branch 'master' into feature/media-features-viewport-characteristics
>>  - Merge branch 'master' into feature/media-features-viewport-characteristics
>>  - ... and 4 more: https://git.openjdk.org/jfx/compare/32e667df...19cdaeaa
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/media/MediaFeatures.java
>  line 250:
> 
>> 248:             case "pc" -> SizeUnits.PC;
>> 249:             default -> throw new IllegalArgumentException(
>> 250:                 String.format("Invalid value <%s> for media feature 
>> <%s>", lowerCaseText, featureName));
> 
> is this a new code?
> could we add a unit test for all the combinations of decimal values (integer, 
> fractional, scientific), with and without the units?

I've removed the custom parsing, and instead delegate to `CssParser` to parse 
sizes. This means we won't have to worry about any of this as part of this PR; 
whatever `CssParser` is doing right or wrong, media queries will use that.

> modules/javafx.graphics/src/test/java/test/javafx/css/CssParser_mediaQuery_Test.java
>  line 675:
> 
>> 673:         assertEquals(1, stylesheet.getRules().size());
>> 674:         assertEquals(
>> 675:             List.of(EqualExpression.of(SizeQueryType.WIDTH, new 
>> Size(100, SizeUnits.PX))),
> 
> Could we add a test that verifies that the CSS parser accepts fractional 
> decimals?  With or without the units specified?
> 
> Also, does it support scientific notation line `1e2` for `100` ?

Even though the CSS accepts scientific notation, `CssLexer` apparently does 
not. This could be a future enhancement.
I've removed all custom code with regards to parsing numbers, and just delegate 
to `CssParser`.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1844#discussion_r2621551123
PR Review Comment: https://git.openjdk.org/jfx/pull/1844#discussion_r2621553367

Reply via email to