On Tue, 2 May 2023 17:57:00 GMT, Jeremy <d...@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
