RFR: JDK-8292276 : Missing color names in CSS. [v10]
ScientificWare
duke at openjdk.org
Fri Sep 2 11:42:42 UTC 2022
On Thu, 1 Sep 2022 20:29:00 GMT, Phil Race <prr at openjdk.org> wrote:
>> ScientificWare has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Adds transparent and RGB case insensitive tests.
>>
>> When <rgb()> and <rgba()> values contains at least an uppercase character, getAttribute returns null.
>> When using 'transparent' keyword getAttribute returns null.
>
> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 1398:
>
>> 1396: static Color stringToColor(String str) {
>> 1397: String strlc = str.toLowerCase(Locale.ROOT);
>> 1398: if (str == null) {
>
> What's the point in testing if it is null after you already de-referenced it ?
Big logical mistake. I going to change the command order.
> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 1411:
>
>> 1409: Color color = colorNamed.get(strlc);
>> 1410: if (color != null) {
>> 1411: return new Color(color.getRGB(), true);
>
> Can you explain why you can't return the color instance directly ?
> They are immutable so I don't see the reason.
To avoid change method behaviour, I tried to reproduce previous behaviors because returned objects can be publicly exposed trough javax.swing.text.html.StyleSheet stringToColor method.
Previous implementation :
When, "" returns always the same object Color.Black.
When, "black", "silver", "white", ..., rgb colors defined ... returns a new object.
I haven't a example list in mind but at least Color1 == Color2 is different according implementation.
> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 1562:
>
>> 1560: Map.entry("tomato", new Color(255, 99, 71, 255)),
>> 1561: Map.entry("transparent", new Color(0, 0, 0, 0)),
>> 1562: Map.entry("turquoise", new Color(64, 224, 208, 255)),
>
> So all of these have the extra 255 alpha arg just so this ONE entry can be different ? Do you expect others like this ? If not why not special case that in the caller method ?
I will correct this.
-------------
PR: https://git.openjdk.org/jdk/pull/9825
More information about the client-libs-dev
mailing list