RFR: 7083187: Class CSS.CssValue is missing implementations of equals() and hashCode() [v15]
Prasanta Sadhukhan
psadhukhan at openjdk.org
Thu Jun 8 03:56:58 UTC 2023
On Wed, 7 Jun 2023 17:11:53 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:
>> No, it doesn't. For some reason, both `100%` and `200%` are parsed so that `span = 1.0`.
>>
>> Let's leave it as is then. It handles the most common case.
>>
>> Handling a space before the percent sign can be postponed to a later fix.
>
> Aha, the value is capped at 100%:
>
> https://github.com/openjdk/jdk/blob/4683844d8a26b56c903f6a67aadb159c81c2a2b8/src/java.desktop/share/classes/javax/swing/text/html/CSS.java#L2627-L2628
>
> This is why 200% is parsed as if it were 100%.
>
> The following code
>
> public boolean equals(Object val) {
> return val instanceof CSS.LengthValue lu
> && percentage == lu.percentage
> && span == lu.span
> && Objects.equals(units, lu.units);
> }
>
> works correctly if you modify this line in the test
>
> https://github.com/openjdk/jdk/blob/4683844d8a26b56c903f6a67aadb159c81c2a2b8/test/jdk/javax/swing/text/html/CSS/CSSAttributeEqualityBug.java#L86
>
> to
>
> {"margin-top: 100%", "margin-top: 50%"},
>
>
> The above code also handles the case `"margin-top: 50 %"` correctly.
I had already made that observation in this [comment ](https://github.com/openjdk/jdk/pull/13405#discussion_r1213916093) few days back in case you overlooked
Also, I kept the percentage check for string even though it fails for "space" within string because it seems "space" is not valid in between % value but we can go beyond 100% ie `50 %` is not valid but `200%` is,
as per https://developer.mozilla.org/en-US/docs/Web/CSS/margin-top
where if you specify `margin-top: 50 %` and then go to other block and come back, you will get a `"X"` but
`margin-top: 200%` is ok
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1222421015
More information about the client-libs-dev
mailing list