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

Prasanta Sadhukhan psadhukhan at openjdk.org
Tue May 23 08:36:05 UTC 2023


On Mon, 22 May 2023 10:59:53 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Test and fix updated as per review comments
>
> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2235:
> 
>> 2233:                 return value == size.value;
>> 2234:             }
>> 2235:             return false;
> 
> Is it not enough to compare `value`, `index` and `lu` fields of `FontSize` object?
> 
> 
> return val instanceof CSS.FontSize size
>        && value == size.value
>        && index == size.index
>        && Objects.equals(lu, size.lu);
> 
> 
> This implies `LengthUnit` implements `equals` but it does not. In your code, `lu` may be `null` which would cause NPE.
> 
> The following code should handle comparing `LengthUnit`s.
> 
> 
> return val instanceof CSS.FontSize size
>        && value == size.value
>        && index == size.index
>        && ( (lu == null && size.lu == null)
>            || (lu != null && size.lu != null
>                && Objects.equals(lu.units, size.lu.units));
> 
> 
> Anyway, parsing the value string does not look good.

The current PR passes regression test along with jtreg/jck tests
The suggested changes above does not pass the regression test

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

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



More information about the client-libs-dev mailing list