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

[Alexey Ivanov]

On Tue, 2 May 2023 11:13:00 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 three additional commits since the last revision:
> 
>  - Test fix
>  - Test fix
>  - Review comment address

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2026:

> 2024:             return (svalue.contains("sup"));
> 2025:         }
> 2026: 

Shall it be reverted?

src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2319:

> 2317:             } else {
> 2318:                 return 0;
> 2319:             }

Suggestion:

            return (family != null) ? family.hashCode() : 0;

The conditional operator is perfect here, don't you think?

src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2321:

> 2319:             }
> 2320:         }
> 2321:         @Override

Let's add a blank line between methods.
Suggestion:


        @Override

test/jdk/javax/swing/text/html/CSS/CSSAttributeEqualityBug.java line 44:

> 42: 
> 43:     private static void testFontSize() {
> 44:         StyleSheet ss = new StyleSheet();

The `StyleSheet` object isn't used in any of the tests, you can safely remove it.

test/jdk/javax/swing/text/html/CSS/CSSAttributeEqualityBug.java line 54:

> 52: 
> 53:         if (a.isEqual(b)) {
> 54:             System.out.println( "a equals b");

Suggestion:

        ss.addCSSAttribute(a, CSS.Attribute.FONT_SIZE, fontSize);

        SimpleAttributeSet b = new SimpleAttributeSet();
        ss.addCSSAttribute(b, CSS.Attribute.FONT_SIZE, fontSize);

        if (a.isEqual(b)) {
            System.out.println("a equals b");

There should be no space after opening parentheses.

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

PR Review: https://git.openjdk.org/jdk/pull/13405#pullrequestreview-1409632838
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1182886952
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1182889142
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1182888179
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1182891734
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1182890669