On Tue, 2 May 2023 11:13:00 GMT, Prasanta Sadhukhan <psadhuk...@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
