On Tue, 30 May 2023 17:32:10 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> 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).
I am not sure I understand...Where the string is parsed...Earlier in my
previous iteration I was using Scanner util class to parse the string to
extract integer 42 and px from 42px and comparing separately which I thought
you were referring to as we should not parse in `equals` so I removed and I
optimized to compare the string value `svalue`
What are you suggesting then? I already told using your suggested code fails
the regression test..
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1211039400
