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

Alexey Ivanov aivanov at openjdk.org
Thu Jun 8 11:13:00 UTC 2023


On Thu, 8 Jun 2023 10:28:17 GMT, Prasanta Sadhukhan <psadhukhan at 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



More information about the client-libs-dev mailing list