On Tue, 2 May 2023 18:10:55 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> 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.

yes this needs to be changed.
Also needs to be changed at line 2034.

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

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

Reply via email to