On Mon, 22 May 2023 10:59:53 GMT, Alexey Ivanov <aiva...@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
