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