Re: RFR: 8202990: javafx webview css filter property with display scaling
On Fri, 28 Aug 2020 05:24:24 GMT, Arun Joseph wrote: >> ImageJava.cpp ignores CompositeOperator parameter in drawImage function due >> to which shadow was getting drawn on top of >> actual image. apply given composite operator to graphics context before >> drawing image to fix this issue. another issue >> is into WCGraphicsPrismContext.java. while blending two layers, applying >> state to the destination layer was missed due >> to which image was not getting drawn with right scale in hidpi mode. apply >> state to fix the issue. > > The fix and test looks good. I spent some time this afternoon going over the fix in more detail and doing extensive testing on both Windows and Mac. I believe the fix is good. Both by inspection and by instrumenting the code, there are no race conditions or other problems that I can see. The problem appears to be in the test, or else somewhere in the test harness or the SW pipeline exposed by the test. If I run the new CSSTest in a loop on either Mac or Windows, it will crash intermittently. I then reverted your fix, running the new test (which will throw an expected assertion error), and it still crashes intermittently. My recommendation is to use a system test, similar to what [SVGTest.java](https://github.com/openjdk/jfx/blob/master/tests/system/src/test/java/test/javafx/scene/web/SVGTest.java) does, rather than a unit test in the javafx.web module, which uses `WebPage::paint`. - PR: https://git.openjdk.java.net/jfx/pull/279
Re: RFR: 8251858: Update to Xcode 11.3.1
On Fri, 28 Aug 2020 18:39:16 GMT, Kevin Rushforth wrote: > This updates the compiler used to build JavaFX on macOS to Xcode 11.3.1 + > MacOSX10.15 sdk, which matches the compiler > used to build JDK 16. > As noted in the bug report, Xcode 11.3.1 needs macOS 10.14.4 (Mojave) or > later to build, but the produced artifacts > will be able to run on earlier versions of macOS. Marked as reviewed by jvos (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/291
RFR: 8251858: Update to Xcode 11.3.1
This updates the compiler used to build JavaFX on macOS to Xcode 11.3.1 + MacOSX10.15 sdk, which matches the compiler used to build JDK 16. As noted in the bug report, Xcode 11.3.1 needs macOS 10.14.4 (Mojave) or later to build, but the produced artifacts will be able to run on earlier versions of macOS. - Commit messages: - 8251858: Update to Xcode 11.3.1 Changes: https://git.openjdk.java.net/jfx/pull/291/files Webrev: https://webrevs.openjdk.java.net/jfx/291/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8251858 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/291.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/291/head:pull/291 PR: https://git.openjdk.java.net/jfx/pull/291
Re: RFR: 8209788: Left/Right/Ctrl+A keys not working in editor of ComboBox if popup showing [v8]
On Fri, 28 Aug 2020 16:45:38 GMT, Ambarish Rapte wrote:
>> The issue occurs because the key events are consumed by the `ListView` in
>> `Popup`, which displays the items.
>> This is a regression of
>> [JDK-8077916](https://bugs.openjdk.java.net/browse/JDK-8077916). This change
>> aadded several
>> `KeyMapping`s for focus traversals to `ListView`, which consume the `Left`,
>> `Right` and `Ctrl+A` key events.
>> Fix:
>> 1. Remove the four focus traversal arrow `KeyMapping`s from
>> `ListViewBehavior`.
>> 2. Add the `Ctrl + A` `KeyMapping` to `ListViewBehavior` only if the
>> `ListView`'s selection mode is set to
>> `SelectionMode.MULTIPLE`. `ComboBox` uses the `ListView` with
>> `SelectionMode.SINGLE` mode.
>> Change unrelated to fix:
>> `ComboBoxListViewBehavior` adds `KeyMapping` for `Up` and `Down` keys, which
>> are not invoked when the `ComboBox` popup
>> is showing. When the popup is shown, the Up and Down key events are handled
>> by the `ListView` and the `KeyMapping` code
>> from `ComboBoxListViewBehavior` does not get executed. These KeyMapping are
>> removed. However this change is not needed
>> for the fix. But this seems to be dead code. Verification:
>> Added new unit tests to verify the change.
>> Also verified that the behavior ListView behaves same before and after this
>> change.
>
> Ambarish Rapte has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix for non editable ComboBox
I haven't tested it, but it looks like it should work. I left a couple of minor
suggestions below.
Would it be possible to add some tests to verify the behavior of HOME and END
for editable and non-editable ComboBox
controls?
modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/ListViewBehavior.java
line 143:
> 142:
> 143: if
> (Boolean.FALSE.equals(control.getProperties().containsKey("excludeKeyMappingsForComboBoxEditor")))
> {
> 144: // This is not ComboBox's ListView
Unless I'm missing something, you don't need to compare with a `Boolean` at all
since containsKey returns a `boolean`
primitive type, so I think this can be simplified to:
if
(!control.getProperties().containsKey("excludeKeyMappingsForComboBoxEditor")) {
If instead you do want to check the value of the property, and not just its
existence, you would need the following,
and it would be important to check `!TRUE.equals` rather than `FALSE.equals` so
that an unset property would be treated
as `false`.
if
(!Boolean.TRUE.equals(control.getProperties().get("excludeKeyMappingsForComboBoxEditor")))
{
modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/ListViewBehavior.java
line 359:
> 358: Boolean isComboBoxEditable =
> (Boolean)getNode().getProperties().get("editableComboBoxEditor");
> 359: if (isComboBoxEditable != null) {
> 360: // This is ComboBox's ListView
Maybe simplify this as follows, to match the earlier logic?
if
(Boolean.FALSE.equals(getNode().getProperties().get("editableComboBoxEditor")))
{
-
PR: https://git.openjdk.java.net/jfx/pull/172
Re: RFR: 8252387: Deprecate for removal css Selector and ShapeConverter constructors [v2]
On Fri, 28 Aug 2020 16:24:54 GMT, Nir Lisker wrote: >> Bhawesh Choudhary has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Updated Selector class comments as per review > > modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 47: > >> 46: * @deprecated This constructor was exposed erroneously and will be >> removed in the next version. >> 47: */ >> 48: @Deprecated(since="16", forRemoval=true) > > Please direct the reader to the way of obtaining an instance of this class > like you did for `ShapeConverter`. Probably you can refer to the `createSelector` method. - PR: https://git.openjdk.java.net/jfx/pull/290
Re: RFR: 8252387: Deprecate for removal css Selector and ShapeConverter constructors [v2]
> Deprecate the public constructor of javafx.css.Selector as it should not be > public due to only being extended by > classes in same package. Deprecate the public constructor of > javafx.css.converter.ShapeConverter as its a singleton > class. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Updated Selector class comments as per review - Changes: - all: https://git.openjdk.java.net/jfx/pull/290/files - new: https://git.openjdk.java.net/jfx/pull/290/files/4719ef18..eb2f71b8 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/290/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/290/webrev.00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/290.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/290/head:pull/290 PR: https://git.openjdk.java.net/jfx/pull/290
Re: RFR: 8209788: Left/Right/Ctrl+A keys not working in editor of ComboBox if popup showing [v7]
On Fri, 28 Aug 2020 10:21:50 GMT, Jeanette Winzenburg
wrote:
> If the ComboBox is not editable, it will have the effect of making the HOME
> and END keys no-ops, which is a (possibly
> unwanted) change in behavior.
I have updated PR with changes for non editable ComboBox.
I could think of adding another property to propagate the editable property of
ComboBox to ListViewBehavior. So now
this fix adds another property `editableComboBoxEditor`, not sure if there is
other way to handle it. The change adds
HOME and END KeyMappings when ComboBox is non editable and removes them when
ComboBox is editable. If the change sounds
Ok, I shall include test in next commit.
Also, there is one change in the if condition that was suggested by Jeanette
before,
`if
(!Boolean.TRUE.equals(control.getProperties().containsKey("excludeKeyMappingsForComboBoxEditor")))`
is changed to,
`if
(Boolean.FALSE.equals(control.getProperties().containsKey("excludeKeyMappingsForComboBoxEditor")))`
It seems safe as `control.getProperties().containsKey()` returns either true or
false.
-
PR: https://git.openjdk.java.net/jfx/pull/172
Re: RFR: 8209788: Left/Right/Ctrl+A keys not working in editor of ComboBox if popup showing [v8]
> The issue occurs because the key events are consumed by the `ListView` in > `Popup`, which displays the items. > This is a regression of > [JDK-8077916](https://bugs.openjdk.java.net/browse/JDK-8077916). This change > aadded several > `KeyMapping`s for focus traversals to `ListView`, which consume the `Left`, > `Right` and `Ctrl+A` key events. > Fix: > 1. Remove the four focus traversal arrow `KeyMapping`s from > `ListViewBehavior`. > 2. Add the `Ctrl + A` `KeyMapping` to `ListViewBehavior` only if the > `ListView`'s selection mode is set to > `SelectionMode.MULTIPLE`. `ComboBox` uses the `ListView` with > `SelectionMode.SINGLE` mode. > Change unrelated to fix: > `ComboBoxListViewBehavior` adds `KeyMapping` for `Up` and `Down` keys, which > are not invoked when the `ComboBox` popup > is showing. When the popup is shown, the Up and Down key events are handled > by the `ListView` and the `KeyMapping` code > from `ComboBoxListViewBehavior` does not get executed. These KeyMapping are > removed. However this change is not needed > for the fix. But this seems to be dead code. Verification: > Added new unit tests to verify the change. > Also verified that the behavior ListView behaves same before and after this > change. Ambarish Rapte has updated the pull request incrementally with one additional commit since the last revision: Fix for non editable ComboBox - Changes: - all: https://git.openjdk.java.net/jfx/pull/172/files - new: https://git.openjdk.java.net/jfx/pull/172/files/22f79d63..2539a5a2 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/172/webrev.07 - incr: https://webrevs.openjdk.java.net/jfx/172/webrev.06-07 Stats: 40 lines in 2 files changed: 38 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/172.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/172/head:pull/172 PR: https://git.openjdk.java.net/jfx/pull/172
Re: RFR: 8252387: Deprecate for removal css Selector and ShapeConverter constructors
On Fri, 28 Aug 2020 14:55:55 GMT, Bhawesh Choudhary wrote: > Deprecate the public constructor of javafx.css.Selector as it should not be > public due to only being extended by > classes in same package. Deprecate the public constructor of > javafx.css.converter.ShapeConverter as its a singleton > class. Looks good. Added a single comment. modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 47: > 46: * @deprecated This constructor was exposed erroneously and will be > removed in the next version. > 47: */ > 48: @Deprecated(since="16", forRemoval=true) Please direct the reader to the way of obtaining an instance of this class like you did for `ShapeConverter`. - PR: https://git.openjdk.java.net/jfx/pull/290
Re: RFR: 8252387: Deprecate for removal css Selector and ShapeConverter constructors
On Fri, 28 Aug 2020 14:55:55 GMT, Bhawesh Choudhary wrote: > Constrcutor for class javafx.css.Selector should not be public as it is only > extended by classes in same package. it > should be changed to package-private Constrcutor for class > javafx.css.converter.ShapeConverter should be private rather > than public as its a singleton class. This is the eventual goal (for JavaFX 17), but it isn't what this bug is addressing for JavaFX 16. Can you reword this to indicate that this bug is just deprecating the constructors for removal, and that we will file a follow-up bug to later change the access? - PR: https://git.openjdk.java.net/jfx/pull/290
RFR: 8252387: Deprecate for removal css Selector and ShapeConverter constructors
Deprecate the public constructor of javafx.css.Selector and javafx.css.converter.ShapeConverter Constrcutor for class javafx.css.Selector should not be public as it is only extended by classes in same package. it should be changed to package-private Constrcutor for class javafx.css.converter.ShapeConverter should be private rather than public as its a singleton class. - Commit messages: - 8252387: Deprecate for removal css Selector and ShapeConverter constructors Changes: https://git.openjdk.java.net/jfx/pull/290/files Webrev: https://webrevs.openjdk.java.net/jfx/290/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252387 Stats: 14 lines in 2 files changed: 14 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/290.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/290/head:pull/290 PR: https://git.openjdk.java.net/jfx/pull/290
Re: 11-dev backport request for 11.0.9: JDK-8252381: Cherry pick GTK WebKit 2.28.4 changes
approved On Fri, Aug 28, 2020 at 3:20 PM Kevin Rushforth wrote: > Hi Johan, > > I request approval to backport the following to 11-dev for 11.0.9: > > JDK-8252381 [1] : Cherry pick GTK WebKit 2.28.4 changes > > -- Kevin > > [1] https://bugs.openjdk.java.net/browse/JDK-8252381 > >
11-dev backport request for 11.0.9: JDK-8252381: Cherry pick GTK WebKit 2.28.4 changes
Hi Johan, I request approval to backport the following to 11-dev for 11.0.9: JDK-8252381 [1] : Cherry pick GTK WebKit 2.28.4 changes -- Kevin [1] https://bugs.openjdk.java.net/browse/JDK-8252381
Integrated: 8252381: Cherry pick GTK WebKit 2.28.4 changes
On Thu, 27 Aug 2020 14:59:56 GMT, Arun Joseph wrote: > Update to GTK WebKit 2.28.4 > https://webkitgtk.org/2020/07/28/webkitgtk2.28.4-released.html This pull request has now been integrated. Changeset: 88c0f978 Author:Arun Joseph URL: https://git.openjdk.java.net/jfx/commit/88c0f978 Stats: 174 lines in 21 files changed: 16 ins; 123 del; 35 mod 8252381: Cherry pick GTK WebKit 2.28.4 changes Reviewed-by: kcr, bchoudhary - PR: https://git.openjdk.java.net/jfx/pull/289
Re: RFR: 8252381: Cherry pick GTK WebKit 2.28.4 changes
On Thu, 27 Aug 2020 14:59:56 GMT, Arun Joseph wrote: > Update to GTK WebKit 2.28.4 > https://webkitgtk.org/2020/07/28/webkitgtk2.28.4-released.html Marked as reviewed by bchoudhary (Author). - PR: https://git.openjdk.java.net/jfx/pull/289
Re: RFR: 8252381: Cherry pick GTK WebKit 2.28.4 changes
On Thu, 27 Aug 2020 21:27:18 GMT, Kevin Rushforth wrote: >> Update to GTK WebKit 2.28.4 >> https://webkitgtk.org/2020/07/28/webkitgtk2.28.4-released.html > > Looks good. Also, I tested on all three platforms. Verified with all three platform. Looks good. - PR: https://git.openjdk.java.net/jfx/pull/289
Re: RFR: 8209788: Left/Right/Ctrl+A keys not working in editor of ComboBox if popup showing [v7]
On Thu, 27 Aug 2020 23:21:28 GMT, Kevin Rushforth wrote: > > > If the ComboBox is not editable, it will have the effect of making the HOME > and END keys no-ops, which is a (possibly > unwanted) change in behavior. I checked a couple native Windows apps and they > have the behavior I would expect: the > arrow keys, and the HOME / END keys navigate the text field for editable > combo boxes. HOME and END go to the beginning > or end of the list for non-editable combo boxes. While we could treat that > as a follow-up issue, it would be worth > thinking about whether we could limit the change to editable combo boxes. outch .. how did I overlook that .. (seems reviewing doesn't belong to my strengths ;) This fix should not break (correct) existing behavior, so back to thinking .. - PR: https://git.openjdk.java.net/jfx/pull/172
Integrated: 8251941: ListCell: visual artifact when items contain null values
On Tue, 25 Aug 2020 11:40:56 GMT, Jeanette Winzenburg wrote: > The issue describes the makroscopic effect/s, namely content showing in cells > that are off range. > > The base reason is missing cleanup of the cell on transition from not-empty > to empty when the old item is a null > contained in the items. Fixed by changing the logic to cope with that special > case. Added tests that fail before and > pass after the fix. This pull request has now been integrated. Changeset: c86bd353 Author:Jeanette Winzenburg URL: https://git.openjdk.java.net/jfx/commit/c86bd353 Stats: 90 lines in 2 files changed: 0 ins; 87 del; 3 mod 8251941: ListCell: visual artifact when items contain null values Reviewed-by: aghaisas - PR: https://git.openjdk.java.net/jfx/pull/288
Integrated: 8251353: Many javafx scenegraph classes have implicit no-arg constructors
On Mon, 17 Aug 2020 11:16:55 GMT, Bhawesh Choudhary wrote: > Added missing explicit no-arg constructors to classes in package > javafx.scene, javafx.css and javafx.stage. This pull request has now been integrated. Changeset: 23ad8f40 Author:Bhawesh Choudhary Committer: Nir Lisker URL: https://git.openjdk.java.net/jfx/commit/23ad8f40 Stats: 80 lines in 12 files changed: 0 ins; 80 del; 0 mod 8251353: Many javafx scenegraph classes have implicit no-arg constructors Reviewed-by: kcr, nlisker - PR: https://git.openjdk.java.net/jfx/pull/283
Re: RFR: 8251941: ListCell: visual artifact when items contain null values
On Tue, 25 Aug 2020 11:40:56 GMT, Jeanette Winzenburg wrote: > The issue describes the makroscopic effect/s, namely content showing in cells > that are off range. > > The base reason is missing cleanup of the cell on transition from not-empty > to empty when the old item is a null > contained in the items. Fixed by changing the logic to cope with that special > case. Added tests that fail before and > pass after the fix. Marked as reviewed by aghaisas (Reviewer). Looks OK. - PR: https://git.openjdk.java.net/jfx/pull/288
