OK, thanks. For the benefit of others who might be interested, this bug
is now visible here:
https://bugs.openjdk.java.net/browse/JDK-8201135
-- Kevin
Matthew Elliot wrote:
Hi Kevin,
Priyanka from Oracle beat me to it and this small example hit the nail
on the head immediately. The below will throw and swallow and
IllegalArgumentException in CSSParser in the following method.
private ParsedValueImpl<Color,Color> colorValueOfString(String str) {
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
public class CssParserTest extends Application {
@Override
public void start(Stage primaryStage) throws Exception {
primaryStage.setTitle("HBox Experiment 1");
Button button1 = new Button("Button Number 1");
Button button2 = new Button("Button Number 2");
VBox vbox = new VBox(button1, button2);
vbox.setStyle("-fx-alignment: top-center;");
Scene scene = new Scene(vbox, 200, 100);
primaryStage.setScene(scene);
primaryStage.show();
}
public static void main(String[] args) {
Application.launch(args);
}
}
There are at least 3 properties I have seen doing this and depending
on the complexity of the CSS, in our case quite extensive it leads to
a lot of throw/catch/ignore. :)
M.
On 4 April 2018 at 18:06, Kevin Rushforth <[email protected]
<mailto:[email protected]>> wrote:
Hi Matt,
Thank you for filing this bug.
Can you provide a standalone test case that reproduces this (a
.java and a .css file), so we can attach it to the bug? Our
WebBugs triage engineer will ask for this, and it will save time
if you can provide it now. Otherwise the bug report looks fine.
-- Kevin
Matthew Elliot wrote:
Hey David, thanks.
I have filed a bug via the Oracle website.
internal review ID : 9053225
Hopefully this was correct as it was also my first time.
Matt
On 4 April 2018 at 17:21, David Grieve
<[email protected] <mailto:[email protected]>> wrote:
On 4/4/18 10:44 AM, Matthew Elliot wrote:
Hi David, thanks for the quick response, the parser does
seem to have
knowledge of the property and values as in the method ...
ParsedValueImpl valueFor(String property, Term root,
CSSLexer lexer) throws ParseException {}
it looks for particular properties for parsing... e.g.
} else if ("-fx-background-color".equals(prop)) {
return parsePaintLayers(root);
} else if ("-fx-background-image".equals(prop)) {
return parseURILayers(root);
} else if ("-fx-background-insets".equals(prop)) {
return parseInsetsLayers(root);
... but fx-alignment and fx-shape for example aren't
listed here and fall
into this strange catch all place I noted in my previous
email.
My follow up questions would be:
1. Why does it hit this for standard css properties as
defined for JavaFX
components(fx-alignment, fx-shape, etc) I.e.
https://docs.oracle.com/
javafx/2/api/javafx/scene/doc-files/cssref.html (hbox,
vbox have
-fx-alignment)
2. Even if it is wanted to be extensible, isn't by default
attempting to
parse a color where the property is not known and
therefore triggering
exception throw / catch on the thread critical to UI perf
a less than
optimal solution? Could it be changed to handle this more
gracefully than
catch / ignore exceptions?
Is it worth raising a ticket for such a topic, would it
ever be considered
for improvement.
I think it is worth raising a ticket.
Thanks again,
Matt
On 4 April 2018 at 16:20, David Grieve
<[email protected] <mailto:[email protected]>>
wrote:
The parser doesn't have any concept of what the
property is or value it
might have. This allows the addition of new properties
(such as an user
might add for their own CSS styles) without having to
modify the parser to
handle them.
On 4/4/18 10:03 AM, Matthew Elliot wrote:
Hi all, (first post).
I was profiling our PROD JavaFX application
recently I discovered
something
rather peculiar in the CSSParser. (jdk1.8.0_151)
I noticed several hundred
IllegalArgumentExceptions on the
JavaApplicationThread where for various unrelated
css properties the
CSSParser is trying to parse a color. While the
exception is subsequently
caught and swallowed silently doing this hundred
of times on this thread
is
rather ugly and caused *minor* delays in the
application thread.
This happened for alignment, shape, and a few
other properties where
no-lookup case was found and it ended on approx.
line 900 of the
CSSParser
in
colorValueOfString()
with a value like 'center'; clearly no color.
// if the property value is another property, then
it needs to be looked
up.
boolean needsLookup = isIdent &&
properties.containsKey(text);
if (needsLookup || ((value =
colorValueOfString(str)) == null )) {
// If the value is a lookup, make sure to use
the lower-case text
so it matches the property
// in the Declaration. If the value is not a
lookup, then use str
since the value might
// be a string which could have some case
sensitive meaning
//
// TODO: isIdent is needed here because of
RT-38345. This
effectively undoes RT-38201
value = new
ParsedValueImpl<String,String>(needsLookup ? text :
str, null, isIdent || needsLookup);
}
I had a look in the bug tracker
https://bugs.openjdk.java.net/ but
didn't
find much in this regard so thought I would post
in case it has come up
before.
I saw some of the css properties are from our
application and some from
e(fx)clipse which I can raise to Tom Schindl
separately if it is a
stylesheet issue, however it would appear that for
example -fx-alignment
in
a layout VBOX/HBOX component should be valid
according to JavaFX docs.
More generally, is it expected that a property
such as -fx-alignment
should
fall into this else {} catch all case, and why
does JavaFX try to parse a
Color by default?
-fx-alignment: center;
Any input much appreciated.
Regards,
Matt
