RE: gradle :web:test fails

[Murali Billa]

Hi Lisker,

Can you print  the value of  useragentString ? Looks like you are using 
fxversion as 11 (you can print this value from  
System.getProperty("javafx.runtime.version");) but the useragentstring does not 
contain 11.

final WebView webView = new WebView();
final WebEngine webEngine = webView.getEngine();
System.out.println(webEngine.getUserAgent()); 


Thanks,
Murali
-Original Message-
From: Nir Lisker  
Sent: Thursday, April 05, 2018 7:03 AM
To: openjfx-dev@openjdk.java.net Mailing 
Subject: gradle :web:test fails

I'm running :web:test in revision 10889 and getting the following failing
test:

test.javafx.scene.web.MiscellaneousTest > testUserAgentString FAILED
java.lang.AssertionError: UserAgentString does not contain JavaFX/11
at org.junit.Assert.fail(Assert.java:91)
at org.junit.Assert.assertTrue(Assert.java:43)
at
test.javafx.scene.web.MiscellaneousTest.lambda$testUserAgentString$12(MiscellaneousTest.java:441)

379 tests completed, 1 failed, 12 skipped :web:test FAILED

Is this known?

- Nir

Re: JDK-8199560: Dynamic evaluation for binding.When

[Nir Lisker]

I don't see any opinions, so should I investigate option 2?

- Nir

On Thu, Mar 29, 2018 at 4:36 AM, Kevin Rushforth  wrote:

> Breaking existing apps is the one thing I worried about with the
> suggestion of doing it without API changes. So to summarize I think there
> are three possiblities:
>
> 1. Just make the current API lazy for observables with no resource for the
> developer. This seems unwise for the reasons you point out.
>
> 2. Keep the existing API, but add new When(ObservableX condition,
> boolean lazyEval) constructors (we can discuss whether the default should
> be eager to preserve compatibility for apps that rely on the side effect of
> eager evaluation or lazy to match what many developers might expect).
>
> 3. Something else.
>
> Unless there is a problem implementing #2 (ignoring the default for now),
> that might be the way to go. It would be good to get some opinions about
> this from other developers.
>
> -- Kevin
>
>
> Nir Lisker wrote:
>
> Iv'e given the idea of using existing API some thought.
>
> When using primitives, the evaluation is always eager. A method that
> returns a boolean (for example) will always be evaluated before it is
> passed to When, regardless of the condition value. This precludes the
> possibility of lazy evaluation in the scenario I've shown in the issue.
>
> When using observables, it's probably possible to do this. The
> observable's value is computed (that is, computeValue() is called) during
> the call to otherwise(...), specifically, when the invalidation listener
> WhenListener is registered on the observable. I think it's possible to
> register and deregister the listener depending on the condition's value.
> Using this approach, a developer can wrap a method call in an observable,
> as done in Bindings.createXxxBinding(Callable...). This is a bit of extra
> work for the developer, having to create a binding. It'll also break
> current behavior unless we do something like adding a constructor
> When(ObservableBooleanValue condition, boolean lazyEval) and default the
> current one to lazyEval=false. We could also use the "we told you so" line
> for developers who rely on the current undocumented behavior.
>
> - Nir
>
> On Sat, Mar 17, 2018 at 1:39 AM, Kevin Rushforth <
> kevin.rushfo...@oracle.com> wrote:
>
>> Conceptually what you describe sounds like a good approach to explore.
>>
>> Another approach worth exploring is to see whether this can be done
>> without API change, using the existing API.  I took a (very quick) look and
>> didn't see anything that would preclude fixing this using the existing API,
>> nor does the specification (javadoc-generated API docs) mandate the current
>> behavior of eagerly evaluating both the "then" and "otherwise" conditions.
>> Since it was only a quick look, I can't be sure.
>>
>> -- Kevin
>>
>>
>> Nir Lisker wrote:
>>
>>> Hello,
>>>
>>> I've proposed to work on a public API for binding.When that adds
>>> capabilities for dynamic evaluation of the 'then' and 'otherwise'
>>> arguments. Any comments?
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8199560
>>>
>>> - Nir
>>>
>>>
>>
>

gradle :web:test fails

[Nir Lisker]

I'm running :web:test in revision 10889 and getting the following failing
test:

test.javafx.scene.web.MiscellaneousTest > testUserAgentString FAILED
java.lang.AssertionError: UserAgentString does not contain JavaFX/11
at org.junit.Assert.fail(Assert.java:91)
at org.junit.Assert.assertTrue(Assert.java:43)
at
test.javafx.scene.web.MiscellaneousTest.lambda$testUserAgentString$12(MiscellaneousTest.java:441)

379 tests completed, 1 failed, 12 skipped
:web:test FAILED

Is this known?

- Nir

RE: Add missing styleable properties to ImageView?

[Pedro Duque Vieira]

 I'd say position, size, color, etc are all part of the design and CSS is
about design so this type of properties should be accessible via CSS.

I don't see why JavaFX should be any different from the web CSS, in this
respect.

There's something I'd like to bring to your attention, we already have
"-fx-pref-width", "-fx-max-width", "-fx-min-width" and the corresponding
height CSS properties coming from Region, should we re-use them? I think it
will be better if we have a standard way to change a Node's size rather
than properties with different names for different Nodes that do the same
thing.
Also having "-fx-pref-width" and also "-fit-width" might be kind of
strange, specially if the former doesn't do anything.
Main point is, I think we should have a standard API (same CSS properties)
for every node if these properties have the same meaning.

Kind regards,


-- 
Pedro Duque Vieira

Re: OpenJFX GitHub mirror

[Kevin Rushforth]

I don't have a strong preference. The URL contains either "issue" or 
"pull" already, so there should be no confusion if we just use the URL.


-- Kevin


Nir Lisker wrote:
github-bug is fine by me. What about external links, name them or 
leave them as URLs? 

On Wed, Apr 4, 2018, 16:43 Johan Vos > wrote:


+1

On Wed, Apr 4, 2018 at 3:30 PM Kevin Rushforth
>
wrote:

Any further comments on this? If not, then I propose we start
using the
JBS label:

github-bug

To identify JBS issues that are linked to guthub issues and/or PRs

-- Kevin


Nir Lisker wrote:
> I think that the labels should be succinct, so one label
should suffice.
> Either github-link or github-bug are fine for me, the latter
because there
> is webbug label already. If a PR needs review just use the
review-request
> existing label.
>
> As for issues and PRs, the issue links section in the JIRA
ticket could
> reflect what exists in GitHub, so there can be 2 web links
named "GitHub
> issue" and "GitHub PR" if need be.
>
> - Nir
>
> On Thu, Mar 29, 2018 at 10:08 AM, Tom Schindl
>
> wrote:
>
>
>> well could we have 2:
>> * github-issue
>> * github-pr
>>
>> The first one indicates someone is working on it over at
github, whereas
>> the second means there's a PR that needs to be review.
>>
>> Tom
>>
>> On 29.03.18 08:42, Laurent Bourgès wrote:
>>
>>> Hi,
>>>
>>> As such github references point to either issue or PR, I
recommend using
>>> the term 'github-link'.
>>>
>>> Laurent
>>>
>>> Le jeu. 29 mars 2018 à 03:40, Kevin Rushforth <
>>>
>> kevin.rushfo...@oracle.com >
>>
>>> a écrit :
>>>
>>>
 I think this would be fine. We would want something that
didn't conflict
 with anything else and wasn't confusing. Possible choices:

 github-link
 github-bug
 gitbug-issue

 Any preferences?

 -- Kevin


 Nir Lisker wrote:

> Kevin, can we get a label for this?
>
> - Nir
>
> On Mon, Mar 26, 2018 at 4:37 PM, Johan Vos
>
>
 wrote:

>
>> Hi Nir,
>>
>> About 4. (jfx-dev): you're right, I just removed that
repository. That
>>
 was

>> just some testing before we did the real thing.
>>
>> As for the other points: I agree
>>
>> - Johan
>>
>> On Mon, Mar 26, 2018 at 12:03 PM Nir Lisker
>
>>
>> wrote:
>>
>>
>>> Hi All,
>>>
>>> A few comments about the mirror and JBS:
>>>
>>> 1. In PRs and issues on GitHub, I strongly suggest
that the link to
>>>
 JBS be

>>> included in the top comment. If the JBS issue was
created after a
>>> discussion, edit it in.
>>>
>>> 2. In JBS, I suggest to link to the GitHub mirror via
More > Link >
>>>
>> Web
>>
>>> Link and in the Link Text use something like "GitHub
mirror" (open
>>>
>> for
>>
>>> suggestions). JIRA renders the link in an easy to see
way (easier
>>>
>> than
>>
>>> looking at URLs). Iv'e tried it in a couple of issues,
e.g.,
>>> https://bugs.openjdk.java.net/browse/JDK-8198795 and
it seems
>>>
 preferable

>>> to
>>> me.
>>>
>>> 3. In JBS, I suggest a new label will be used for
issues which are
>>>
 linked

>>> to GitHub for search purposes. This is similar to the
webbug label.
>>>
>>> If these are agreeable, please add them to the
contribution
>>>
 instructions.

>>> 4. What is 

Re: OpenJFX GitHub mirror

[Nir Lisker]

github-bug is fine by me. What about external links, name them or leave
them as URLs?

On Wed, Apr 4, 2018, 16:43 Johan Vos  wrote:

> +1
>
> On Wed, Apr 4, 2018 at 3:30 PM Kevin Rushforth 
> wrote:
>
>> Any further comments on this? If not, then I propose we start using the
>> JBS label:
>>
>> github-bug
>>
>> To identify JBS issues that are linked to guthub issues and/or PRs
>>
>> -- Kevin
>>
>>
>> Nir Lisker wrote:
>> > I think that the labels should be succinct, so one label should suffice.
>> > Either github-link or github-bug are fine for me, the latter because
>> there
>> > is webbug label already. If a PR needs review just use the
>> review-request
>> > existing label.
>> >
>> > As for issues and PRs, the issue links section in the JIRA ticket could
>> > reflect what exists in GitHub, so there can be 2 web links named "GitHub
>> > issue" and "GitHub PR" if need be.
>> >
>> > - Nir
>> >
>> > On Thu, Mar 29, 2018 at 10:08 AM, Tom Schindl <
>> tom.schi...@bestsolution.at>
>> > wrote:
>> >
>> >
>> >> well could we have 2:
>> >> * github-issue
>> >> * github-pr
>> >>
>> >> The first one indicates someone is working on it over at github,
>> whereas
>> >> the second means there's a PR that needs to be review.
>> >>
>> >> Tom
>> >>
>> >> On 29.03.18 08:42, Laurent Bourgès wrote:
>> >>
>> >>> Hi,
>> >>>
>> >>> As such github references point to either issue or PR, I recommend
>> using
>> >>> the term 'github-link'.
>> >>>
>> >>> Laurent
>> >>>
>> >>> Le jeu. 29 mars 2018 à 03:40, Kevin Rushforth <
>> >>>
>> >> kevin.rushfo...@oracle.com>
>> >>
>> >>> a écrit :
>> >>>
>> >>>
>>  I think this would be fine. We would want something that didn't
>> conflict
>>  with anything else and wasn't confusing. Possible choices:
>> 
>>  github-link
>>  github-bug
>>  gitbug-issue
>> 
>>  Any preferences?
>> 
>>  -- Kevin
>> 
>> 
>>  Nir Lisker wrote:
>> 
>> > Kevin, can we get a label for this?
>> >
>> > - Nir
>> >
>> > On Mon, Mar 26, 2018 at 4:37 PM, Johan Vos 
>> >
>>  wrote:
>> 
>> >
>> >> Hi Nir,
>> >>
>> >> About 4. (jfx-dev): you're right, I just removed that repository.
>> That
>> >>
>>  was
>> 
>> >> just some testing before we did the real thing.
>> >>
>> >> As for the other points: I agree
>> >>
>> >> - Johan
>> >>
>> >> On Mon, Mar 26, 2018 at 12:03 PM Nir Lisker 
>> >>
>> >> wrote:
>> >>
>> >>
>> >>> Hi All,
>> >>>
>> >>> A few comments about the mirror and JBS:
>> >>>
>> >>> 1. In PRs and issues on GitHub, I strongly suggest that the link
>> to
>> >>>
>>  JBS be
>> 
>> >>> included in the top comment. If the JBS issue was created after a
>> >>> discussion, edit it in.
>> >>>
>> >>> 2. In JBS, I suggest to link to the GitHub mirror via More > Link
>> >
>> >>>
>> >> Web
>> >>
>> >>> Link and in the Link Text use something like "GitHub mirror" (open
>> >>>
>> >> for
>> >>
>> >>> suggestions). JIRA renders the link in an easy to see way (easier
>> >>>
>> >> than
>> >>
>> >>> looking at URLs). Iv'e tried it in a couple of issues, e.g.,
>> >>> https://bugs.openjdk.java.net/browse/JDK-8198795 and it seems
>> >>>
>>  preferable
>> 
>> >>> to
>> >>> me.
>> >>>
>> >>> 3. In JBS, I suggest a new label will be used for issues which are
>> >>>
>>  linked
>> 
>> >>> to GitHub for search purposes. This is similar to the webbug
>> label.
>> >>>
>> >>> If these are agreeable, please add them to the contribution
>> >>>
>>  instructions.
>> 
>> >>> 4. What is https://github.com/javafxports/jfx-dev? Newcomers can
>> >>>
>>  confuse
>> 
>> >>> it
>> >>> with javafxports/openjdk-jfx.
>> >>>
>> >>> 5. When the mirror is fully ready and operational, we should
>> >>>
>> >> advertise
>> >>
>>  on
>> 
>> >>> community pages (like Reddit) to gather up contributors. Please
>> keep
>> >>>
>> >> a
>> >>
>> >>> mental reminder when the time comes.
>> >>>
>> >>> Thanks to all who are working on this.
>> >>>
>> >>> - Nir
>> >>>
>> >>>
>> >>>
>>
>

Re: CSSParser Color.parse() for unexpected CSS properties

[Kevin Rushforth]

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


  

Re: CSSParser Color.parse() for unexpected CSS properties

[Matthew Elliot]

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

Re: CSSParser Color.parse() for unexpected CSS properties

[David Grieve]

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

Re: CSSParser Color.parse() for unexpected CSS properties

[Matthew Elliot]

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.

Thanks again,
Matt


On 4 April 2018 at 16:20, David Grieve  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(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
>>
>
>

Re: CSSParser Color.parse() for unexpected CSS properties

[David Grieve]

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

CSSParser Color.parse() for unexpected CSS properties

[Matthew Elliot]

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

Re: OpenJFX GitHub mirror

[Johan Vos]

+1

On Wed, Apr 4, 2018 at 3:30 PM Kevin Rushforth 
wrote:

> Any further comments on this? If not, then I propose we start using the
> JBS label:
>
> github-bug
>
> To identify JBS issues that are linked to guthub issues and/or PRs
>
> -- Kevin
>
>
> Nir Lisker wrote:
> > I think that the labels should be succinct, so one label should suffice.
> > Either github-link or github-bug are fine for me, the latter because
> there
> > is webbug label already. If a PR needs review just use the review-request
> > existing label.
> >
> > As for issues and PRs, the issue links section in the JIRA ticket could
> > reflect what exists in GitHub, so there can be 2 web links named "GitHub
> > issue" and "GitHub PR" if need be.
> >
> > - Nir
> >
> > On Thu, Mar 29, 2018 at 10:08 AM, Tom Schindl <
> tom.schi...@bestsolution.at>
> > wrote:
> >
> >
> >> well could we have 2:
> >> * github-issue
> >> * github-pr
> >>
> >> The first one indicates someone is working on it over at github, whereas
> >> the second means there's a PR that needs to be review.
> >>
> >> Tom
> >>
> >> On 29.03.18 08:42, Laurent Bourgès wrote:
> >>
> >>> Hi,
> >>>
> >>> As such github references point to either issue or PR, I recommend
> using
> >>> the term 'github-link'.
> >>>
> >>> Laurent
> >>>
> >>> Le jeu. 29 mars 2018 à 03:40, Kevin Rushforth <
> >>>
> >> kevin.rushfo...@oracle.com>
> >>
> >>> a écrit :
> >>>
> >>>
>  I think this would be fine. We would want something that didn't
> conflict
>  with anything else and wasn't confusing. Possible choices:
> 
>  github-link
>  github-bug
>  gitbug-issue
> 
>  Any preferences?
> 
>  -- Kevin
> 
> 
>  Nir Lisker wrote:
> 
> > Kevin, can we get a label for this?
> >
> > - Nir
> >
> > On Mon, Mar 26, 2018 at 4:37 PM, Johan Vos 
> >
>  wrote:
> 
> >
> >> Hi Nir,
> >>
> >> About 4. (jfx-dev): you're right, I just removed that repository.
> That
> >>
>  was
> 
> >> just some testing before we did the real thing.
> >>
> >> As for the other points: I agree
> >>
> >> - Johan
> >>
> >> On Mon, Mar 26, 2018 at 12:03 PM Nir Lisker 
> >>
> >> wrote:
> >>
> >>
> >>> Hi All,
> >>>
> >>> A few comments about the mirror and JBS:
> >>>
> >>> 1. In PRs and issues on GitHub, I strongly suggest that the link to
> >>>
>  JBS be
> 
> >>> included in the top comment. If the JBS issue was created after a
> >>> discussion, edit it in.
> >>>
> >>> 2. In JBS, I suggest to link to the GitHub mirror via More > Link >
> >>>
> >> Web
> >>
> >>> Link and in the Link Text use something like "GitHub mirror" (open
> >>>
> >> for
> >>
> >>> suggestions). JIRA renders the link in an easy to see way (easier
> >>>
> >> than
> >>
> >>> looking at URLs). Iv'e tried it in a couple of issues, e.g.,
> >>> https://bugs.openjdk.java.net/browse/JDK-8198795 and it seems
> >>>
>  preferable
> 
> >>> to
> >>> me.
> >>>
> >>> 3. In JBS, I suggest a new label will be used for issues which are
> >>>
>  linked
> 
> >>> to GitHub for search purposes. This is similar to the webbug label.
> >>>
> >>> If these are agreeable, please add them to the contribution
> >>>
>  instructions.
> 
> >>> 4. What is https://github.com/javafxports/jfx-dev? Newcomers can
> >>>
>  confuse
> 
> >>> it
> >>> with javafxports/openjdk-jfx.
> >>>
> >>> 5. When the mirror is fully ready and operational, we should
> >>>
> >> advertise
> >>
>  on
> 
> >>> community pages (like Reddit) to gather up contributors. Please
> keep
> >>>
> >> a
> >>
> >>> mental reminder when the time comes.
> >>>
> >>> Thanks to all who are working on this.
> >>>
> >>> - Nir
> >>>
> >>>
> >>>
>

Re: OpenJFX GitHub mirror

[Kevin Rushforth]

Any further comments on this? If not, then I propose we start using the 
JBS label:


   github-bug

To identify JBS issues that are linked to guthub issues and/or PRs

-- Kevin


Nir Lisker wrote:

I think that the labels should be succinct, so one label should suffice.
Either github-link or github-bug are fine for me, the latter because there
is webbug label already. If a PR needs review just use the review-request
existing label.

As for issues and PRs, the issue links section in the JIRA ticket could
reflect what exists in GitHub, so there can be 2 web links named "GitHub
issue" and "GitHub PR" if need be.

- Nir

On Thu, Mar 29, 2018 at 10:08 AM, Tom Schindl 
wrote:

  

well could we have 2:
* github-issue
* github-pr

The first one indicates someone is working on it over at github, whereas
the second means there's a PR that needs to be review.

Tom

On 29.03.18 08:42, Laurent Bourgès wrote:


Hi,

As such github references point to either issue or PR, I recommend using
the term 'github-link'.

Laurent

Le jeu. 29 mars 2018 à 03:40, Kevin Rushforth <
  

kevin.rushfo...@oracle.com>


a écrit :

  

I think this would be fine. We would want something that didn't conflict
with anything else and wasn't confusing. Possible choices:

github-link
github-bug
gitbug-issue

Any preferences?

-- Kevin


Nir Lisker wrote:


Kevin, can we get a label for this?

- Nir

On Mon, Mar 26, 2018 at 4:37 PM, Johan Vos 
  

wrote:

  

Hi Nir,

About 4. (jfx-dev): you're right, I just removed that repository. That


was


just some testing before we did the real thing.

As for the other points: I agree

- Johan

On Mon, Mar 26, 2018 at 12:03 PM Nir Lisker 


wrote:



Hi All,

A few comments about the mirror and JBS:

1. In PRs and issues on GitHub, I strongly suggest that the link to
  

JBS be


included in the top comment. If the JBS issue was created after a
discussion, edit it in.

2. In JBS, I suggest to link to the GitHub mirror via More > Link >
  

Web


Link and in the Link Text use something like "GitHub mirror" (open
  

for


suggestions). JIRA renders the link in an easy to see way (easier
  

than


looking at URLs). Iv'e tried it in a couple of issues, e.g.,
https://bugs.openjdk.java.net/browse/JDK-8198795 and it seems
  

preferable


to
me.

3. In JBS, I suggest a new label will be used for issues which are
  

linked


to GitHub for search purposes. This is similar to the webbug label.

If these are agreeable, please add them to the contribution
  

instructions.


4. What is https://github.com/javafxports/jfx-dev? Newcomers can
  

confuse


it
with javafxports/openjdk-jfx.

5. When the mirror is fully ready and operational, we should
  

advertise


on


community pages (like Reddit) to gather up contributors. Please keep
  

a


mental reminder when the time comes.

Thanks to all who are working on this.

- Nir


  

[11] Review request : JDK-8185854 : [JavaFX 9] NPE on non-editable ComboBox in TabPane with custom Skin

[Ajit Ghaisas]

Hi,

Please review below fix.

Bug :  https://bugs.openjdk.java.net/browse/JDK-8185854
Fix :  http://cr.openjdk.java.net/~aghaisas/fx/8185854/webrev.0/

Request you to review.

Regards,
Ajit

RE: Add missing styleable properties to ImageView?

[Prem Balakrishnan]

Hi Johan,

I was not aware of this PR and hence worked on this RFE separately.
Thanks for bringing it to my notice.

I am watching the thread now and I will let Andres take it forward.

Regards,
Prem

-Original Message-
From: Johan Vos  
Sent: Tuesday, April 03, 2018 6:16 PM
To: openjfx-dev@openjdk.java.net List 
Subject: Add missing styleable properties to ImageView?

There is a discussion on an issue at github whether some properties on 
ImageView need to be css-styleable:
https://github.com/javafxports/openjdk-jfx/issues/29

As not everybody is checking those issues, I want to bring this under the 
attention of the list, as it has important consequences (once we do this, what 
other properties might follow?)

There is a related PR at https://github.com/javafxports/openjdk-jfx/pull/30

I don't have a particular opinion on this topic, but I want to hear what others 
think.

- Johan