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

[Alexey Ivanov]

On Mon, 10 Apr 2023 12:44:08 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

The change looks good but it doesn't address the whole problem raised in the [JDK-7083187](https://bugs.openjdk.org/browse/JDK-7083187): `CSS.CssValue` does not implement `equals`. This fix addresses only one particular case: `CSS.FontSize` for `font-size` property.

I do not think it resolves the problem *entirely*: `CssValue` and all its subclasses must implement `equals` method, otherwise adding another CSS attribute to `AttributeSet` will lead to this same issue described in the bug report.

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

> 2201:         }
> 2202: 
> 2203:         public int hashCode() {

You should add `@Override` annotation and to `equals` method too.

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

> 2211:                 return false;
> 2212:             }
> 2213:             return false;

Suggestion:

            return val instanceof CSS.FontSize size && value == size.value;


I won't insist though.

test/jdk/javax/swing/text/html/CSS/CSSBug.java line 25:

> 23: /*
> 24:  * @test
> 25:  * @key headful

Does it need to be headful?

test/jdk/javax/swing/text/html/CSS/CSSBug.java line 34:

> 32: import javax.swing.text.html.StyleSheet;
> 33: 
> 34: public class CSSBug {

Could this be a more specific name? I believe the agreement is giving descriptive test names, right?

test/jdk/javax/swing/text/html/CSS/CSSBug.java line 47:

> 45: 
> 46:         if (a.isEqual( b) ) {
> 47:             System.out.println( "a equals b");

Could you please the remove the extra spaces after and before the parentheses.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13405#pullrequestreview-1381636095
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1164340214
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1164341958
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1164342718
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1164344571
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1164345588