On Tue, 30 May 2023 17:18:48 GMT, Prasanta Sadhukhan <[email protected]>
wrote:
>> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2225:
>>
>>> 2223: return val instanceof CSS.FontSize size
>>> 2224: && Objects.equals(size.svalue, svalue);
>>> 2225: }
>>
>> This is an anomaly. In all the other cases below you are using the local
>> fields not the original parsed string.
>
> This is because font-size attribute can have units or percentage in it, it
> seems like as is being tested in the regression test, 42px, 42pt so "svalue"
> is being used which will have 42px/42pt so we can compare the string value of
> 42px or 42pt otherwise we need to parse to get the integer 42 and then the
> units px or pt and then compared separately
We should not parse the string, it's already parsed.
What if I put a space between value and unit? These two strings should produce
equal attribute sets:
{"font-size: 42px", "font-size: 42 px"},
The easiest way is to implement `LengthUnit.equals` which you still need for
comparing `CSS.LengthValue` anyway because that class has exactly the same
problem. You also compare the raw string there:
https://github.com/openjdk/jdk/blob/612d5eba4c83ab53c797b03a244879261a5faa2a/src/java.desktop/share/classes/javax/swing/text/html/CSS.java#L2678-L2681
It doesn't look right to me.
This is basically [a continuation of the same
discussion](https://github.com/openjdk/jdk/pull/13405#discussion_r1202100159).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1210602634
