RFR: 7083187: Class CSS.CssValue is missing implementations of equals() and hashCode() [v15]
Prasanta Sadhukhan
psadhukhan at openjdk.org
Thu Jun 8 07:01:59 UTC 2023
On Thu, 8 Jun 2023 06:47:38 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:
>> I had already made that observation in this [comment ](https://github.com/openjdk/jdk/pull/13405#discussion_r1213916093) few days back in case you overlooked
>> Also, I kept the percentage check for string even though it fails for "space" within string because it seems "space" is not valid in between % value but we can go beyond 100% ie `50 %` is not valid but `200%` is,
>> as per https://developer.mozilla.org/en-US/docs/Web/CSS/margin-top
>> where if you specify `margin-top: 50 %` and then go to other block and come back, you will get a `"X"` but
>> `margin-top: 200%` is ok
>
>> I had already made that observation in this https://github.com/openjdk/jdk/pull/13405#discussion_r1213916093few days back in case you overlooked
>
> I did miss this comment. Sorry about that.
>
> Even though space between the value and the percent sign or the units is invalid (I couldn't find it quickly in the W3C spec for CSS), you should compare the parsed values. We pass `100%` and `200%` but the parsed value is `100%` in both cases — **the attribute sets are indeed *equal***. If you apply either attribute set to a document, you'll get the same result. Does it make sense?
>
> There could be other cases where the computed/parsed values are the same even though the input is different, for example `"font-size: medium"` has a numeric value in points or pixels, so the attribute set with the same value should be equal, don't you agree?
I am not sure on 100%, 200% ie >= 100% should be considered equal or not..I could not find in spec...also the URL I gave has 200% as valid value...so as of now I have considered what is normal and made `equals` return false for different percentages irrespective of < or > 100%..
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1222543625
More information about the client-libs-dev
mailing list