On Wed, 28 Sep 2022 17:13:42 GMT, ScientificWare <d...@openjdk.org> wrote:
>> This is referenced in Java Bug Database as >> - [JDK-8292276 : Missing color names in >> CSS](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8292276) >> >> This is tracked in JBS as >> - [JDK-8292276 : Missing color names in >> CSS](https://bugs.openjdk.java.net/browse/JDK-8292276) >> >> Adds missing color names, defined by CSS Level 4, in CSS.java : >> CSS Color Module Level 4 >> W3C Candidate Recommendation Snapshot, 5 July 2022 >> [7.1 Named Colors](https://www.w3.org/TR/css-color-4/#named-color) >> >> Designed from : [ScientificWare JDK-8292276 : Missing color names in >> CSS](https://github.com/scientificware/jdk/issues/12) > > ScientificWare has updated the pull request incrementally with one additional > commit since the last revision: > > Moves jtreg tags to the class declaration > > Comment with tags isn't collapsed when viewed in an IDE if it's placed > before the class declaration. > > Are you talking about [#9825 > > (comment)](https://github.com/openjdk/jdk/pull/9825#issuecomment-1216011827)? > > @SWinxy Yes, only because the result is publicly exposed : `stringToColor` is > also used by `javax.swing.text.html.StyleSheet stringToColor` method to > publicly return the `Color` Object. It seems unique, I didn't see any other > usages. > > > also reminder that .contains...(Object) exists > > I saw these methods but in our case we need a result anyway. Using > `containsKey(Object)` implies a second request to get the value ? I thought we had closed [this discussion](https://github.com/openjdk/jdk/pull/9825#issuecomment-1216011827): A `Color` object is immutable, it is safe to return it as it's stored in the internal map. The fact that the previous implementation returned a new instance each time `stringToColor` was called is an implementation detail, no one should have relied on it. @scientificware is right, we need the result anyway, so the code in the PR is more efficient: https://github.com/openjdk/jdk/pull/9825/files#diff-e2c9b23b1844fa877fb1c4f048a8a8b85cd21d146f963837e039ab20b9560fe7R1409-R1412 It looks up a color in the map once whereas if (colorNamed.containsKey(str.toLowerCase(Locale.ROOT))) { return colorNamed.get(str.toLowerCase(Locale.ROOT)); } performs the look up twice: for `containsKey` and for `get`. ------------- PR: https://git.openjdk.org/jdk/pull/9825