On Sun, 12 Feb 2023 08:59:42 GMT, ScientificWare <d...@openjdk.org> wrote:
>> This is referenced in Java Bug Database as >> - [JDK-8292276 : Add named colors from CSS Color Module Level >> 4](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8292276) >> >> This is tracked in JBS as >> - [JDK-8292276 : Add named colors from CSS Color Module Level >> 4](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 : Add named colors from CSS >> Color Module Level 4](https://github.com/scientificware/jdk/issues/12) > > ScientificWare has updated the pull request incrementally with one additional > commit since the last revision: > > Delete configure > > Delete configure from the pull request. Changes requested by aivanov (Reviewer). src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 1416: > 1414: } > 1415: // sometimes get specified without leading # > 1416: return hexToColor(str); I can't see the definitions of `parseRGB`, `parseRGBA`, `hexToColor`. src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 3780: > 3778: > 3779: static int baseFontSizeIndex = 3; > 3780: } Please preserve the new line char in the end of files. src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java line 1019: > 1017: * Converts a color string such as "RED", "rgb(r g b)", > 1018: * "rgba(r g b a)" or "#NNN", "#NNNN", "#NNNNNN", > 1019: * "#NNNNNNNN" to a Color. [CSS Color Level 4, The RGB functions](https://www.w3.org/TR/css-color-4/#rgb-functions) specifies that both `rgb` and `rgba` have the same syntax and both accept `<alpha-value>`. src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java line 1025: > 1023: * and rgb or rgba HTML3.2 strings. > 1024: * Otherwise, it will return null. > 1025: * This method is case-insensitive. Suggestion: * Note: This will only convert strings which use any of the following: * <ul> * <li><a href="https://www.w3.org/TR/css-color-4/#named-colors";>named colors</a>, * <li>hex color notation starting with {@code #} followed by 3, 4, 6, or 8 hexadecimal digits, * <li>`rgb()` and `rgba()` functions * </ul> * as specified by the <a href="https://www.w3.org/TR/css-color-4/";>CSS Color Module Level 4</a>. * Otherwise, it will return null. * <p> * This method is case-insensitive. src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java line 1027: > 1025: * This method is case-insensitive. > 1026: * <p> > 1027: * The following code defines instances of the same color : Suggestion: * The following code defines instances of the same color: There should be no space before the colon. src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java line 1030: > 1028: * {@snippet lang="java" : > 1029: * import java.awt.Color; > 1030: * import javax.swing.text.html.StyleSheet; Probably, the imports could be omitted for the sake of brevity. test/jdk/javax/swing/text/html/CSS/MissingColorNames.java line 62: > 60: passed = false; > 61: result.append(" [<name-color> keyword(s) missing]"); > 62: } Does it make sense to add a couple of named colors, especially from a _more_ extended set? test/jdk/javax/swing/text/html/CSS/MissingColorNames.java line 70: > 68: passed = false; > 69: result.append(" [<rgb()> or <rgba()> values not case > insensitive]"); > 70: } I'd rather see the test demonstrates all the supported syntax for defining color using `rgb()` and `rgba()` functions as well as for `#N+` notations so that we're sure they work as specified by the CSS specification. It could be split into several test cases which use `StyleSheet.stringToColor` method for parsing. I know this would look like a unit test than a regression test. Yet you still have to write one to verify your code works as expected, and the unit test has an intrinsic value: if the code is modified for whatever reason, it ensures all the cases still produce the expected result. ------------- PR: https://git.openjdk.org/jdk/pull/9825
