On Thu, 8 Jun 2023 10:28:17 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org>
wrote:
>> Two CSS AttributeSet-s can be compared using the AttributeSet.isEqual()
>> method which can fail due to missing implementation of equals method in CSS
>> subclasses.
>> In this issue, even when two CSS AttributeSet has same 42 font size string
>> value, Object equality fails.
>> Fixed by implementing the equality and hashCode method for CSS.FontSize
>> class.
>>
>> All jtreg/jck tests are ok
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix percentage equality as per span parsed value
Thank you for updating the code.
src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2223:
> 2221: }
> 2222: return hashCode;
> 2223: }
To be consistent with `LengthValue` and `LengthUnit`:
Suggestion:
public int hashCode() {
return Float.hashCode(value)
| Boolean.hashCode(index)
| Objects.hashCode(lu);
}
src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2690:
> 2688: return val instanceof CSS.LengthValue lu
> 2689: && span == lu.span
> 2690: && Objects.equals(units, lu.units);
[The `percentage`
field](https://github.com/openjdk/jdk/pull/13405#discussion_r1221932524) must
also be part of `equals`:
Suggestion:
return val instanceof CSS.LengthValue lu
&& percentage == lu.percentage
&& span == lu.span
&& Objects.equals(units, lu.units);
You have included it in `hashCode`.
src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 3090:
> 3088: public int hashCode() {
> 3089: return Float.hashCode(value) | Short.hashCode(type)
> 3090: | Objects.hashCode(units);
To be consistent with other similar methods above:
Suggestion:
return Float.hashCode(value)
| Short.hashCode(type)
| Objects.hashCode(units);
-------------
Changes requested by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13405#pullrequestreview-1469660047
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1222878939
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1222884735
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1222886163
