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]> 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]> 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 >>> >> >> > >
