RFR: 7083187: Class CSS.CssValue is missing implementations of equals() and hashCode() [v15]

Alexey Ivanov aivanov at openjdk.org
Thu Jun 8 06:51:01 UTC 2023


On Thu, 8 Jun 2023 03:52:38 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> 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

> I had already made that observation in this https://github.com/openjdk/jdk/pull/13405#discussion_r1213916093few days back in case you overlooked

I did miss this comment. Sorry about that.

Even though space between the value and the percent sign or the units is invalid (I couldn't find it quickly in the W3C spec for CSS), you should compare the parsed values. We pass `100%` and `200%` but the parsed value is `100%` in both cases — **the attribute sets are indeed *equal***. If you apply either attribute set to a document, you'll get the same result. Does it make sense?

There could be other cases where the computed/parsed values are the same even though the input is different, for example `"font-size: medium"` has a numeric value in points or pixels, so the attribute set with the same value should be equal, don't you agree?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1222531449



More information about the client-libs-dev mailing list