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

Alexey Ivanov aivanov at openjdk.org
Tue May 2 18:13:18 UTC 2023


On Tue, 2 May 2023 17:57:00 GMT, Jeremy <duke at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Test fix
>>  - Test fix
>>  - Review comment address
>
> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2323:
> 
>> 2321:         @Override
>> 2322:         public boolean equals(Object val) {
>> 2323:             return val instanceof CSS.FontFamily font && family == font.family;
> 
> Is the `family` field interned somewhere? If it isn't, then should this be:
> 
> 
> return val instanceof CSS.FontFamily font && 
>     Objects.equals(family, font.family);
> 
> 
> For ex if we tweak the start of the FontFamily unit test as follows does it still pass?
> 
> private static void testFontFamily() {
>     StyleSheet ss = new StyleSheet();
> 
>     SimpleAttributeSet a = new SimpleAttributeSet();
>     ss.addCSSAttribute( a, CSS.Attribute.FONT_FAMILY, "Sans-Serif");
> 
>     SimpleAttributeSet b = new SimpleAttributeSet();
>     ss.addCSSAttribute( b, CSS.Attribute.FONT_FAMILY, "Sans-Serif");

I'm pretty sure the test will still pass because Java compiler interns strings. You have to explicitly create a new instance of the String to make the test fail.

In real applications, attributes are parsed from a style sheet and therefore the identity test won't pass.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1182886410



More information about the client-libs-dev mailing list