Re: RFR: 8299423: JavaFX Mac system menubar leaks [v5]

2023-01-16 Thread Michael Hall 


> On Jan 16, 2023, at 10:33 PM, Scott Palmer  wrote:
> 
> There are third-party libraries for better integration with the Mac menu bar.
> 
> E.g. https://github.com/codecentric/NSMenuFX 
> 
> 
> Though I wish this wasn’t necessary and proper hooks were present in JavaFX 
> to begin with.
> 

That does look like it provides the About menu item. In looking at this I 
didn’t determine why JavaFX app’s don’t get the default ‘Cocoa’ one Swing app’s 
do (free of charge, no need for java.awt.Desktop or any app changes at all). 
This should just work. If the changes I provided in the bug report were used 
you would get this by simply including…

static {
//java.awt.Toolkit.getDefaultToolkit(); // Start AppKit
Thread t = new Thread(() -> { 
java.awt.Toolkit.getDefaultToolkit(); });
t.start();  
}

The thread change seemed necessary to hit ApplicationDelegate with access to 
the menubar. I don’t know yet if it will be considered in the scope of the bug 
fix to determine why the code doesn’t work as-is and eliminate the need for 
this.

I think originally the setAboutHandler and other functionalities of 
java.awt.Desktop were the jdk’s attempt to get rid of the equivalent Apple 
API’s as a sort of 3rd party add on. I wondered if it might not be more 
generally useful for JavaFX to have full access to java.awt.Desktop. I did go 
so far as to verify that with the following…

public class HelloWorld extends Application implements AboutHandler {

@Override
public void init() {
Thread t = new Thread(() -> {
Desktop desktop = Desktop.getDesktop();
desktop.setAboutHandler((AboutHandler)this);   
});
t.start();  
}

public void handleAbout(AboutEvent evt) {
System.out.println("got to handleAbout");
Platform.runLater(() -> {
Alert alert = new Alert(AlertType.INFORMATION, "About 
HelloWorld");
alert.showAndWait();
});
}

Again, the thread changes seemed necessary. 
I don’t make much other use of java.awt.Desktop anywhere myself. But possibly 
other JavaFX developers would find some of it’s functionality useful or 
necessary. So I think long term some determination might need to made if JavaFX 
will develop all of this themselves. Or, use of java.awt.Desktop can continue 
to be used. Whether any attempt to address that will be made in resolving the 
bug report I don’t know. From the jdk side they could possibly figure out some 
way to provide this without some of the Thread changes. Or indicate if any 
support for java.awt.Desktop for JavaFX applications will be made at all.

Re: RFR: 8273379 - GTK3 stops sending key events during drag and drop [v21]

2023-01-16 Thread Thiago Milczarek Sayao 
> This PR fixes 8273379.
> 
> I reverted back to use GDK (from 
> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the events. 
> 
> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
> 
> There's also some cleaup.
> 
> To do general testing (two tests were added):
> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`
> 
> Information for reviewing:
> * Previously an offscreen window where used to pass events. Now it gets the 
> window were Drag initially started  
> (`WindowContextBase::sm_mouse_drag_window`);
> * There's a new `DragSourceContext` instead of global variables;
> * DragView were simplified;
> * It now uses a event hook when drag starts to listen to Drag/DragView events;
> * It handles `GDK_GRAB_BROKEN` events (I still need to figure it out a test 
> case for this - It should happen when another window grabs the device during 
> the drag);
> * It uses `gdk_threads_add_idle_full` to end drag so the `GDK_BUTTON_RELEASE` 
> event will continue before ending the drag. This is needed because 
> `WindowContext` will notify java about the button release;
> * It also notifies "Drag Dropped" on DND Source if the drop is on the same 
> app, because it needs to do so before mouse button is released - this method 
> is more straightforward;
> * The Scenario were the drag source window closes during the drag is now 
> covered;

Thiago Milczarek Sayao has updated the pull request incrementally with one 
additional commit since the last revision:

  Remove commented out code

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/986/files
  - new: https://git.openjdk.org/jfx/pull/986/files/04c4c954..aa5d8ae2

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=20
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=19-20

  Stats: 9 lines in 1 file changed: 0 ins; 9 del; 0 mod
  Patch: https://git.openjdk.org/jfx/pull/986.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/986/head:pull/986

PR: https://git.openjdk.org/jfx/pull/986

Re: RFR: 8273379 - GTK3 stops sending key events during drag and drop [v20]

2023-01-16 Thread Thiago Milczarek Sayao 
> This PR fixes 8273379.
> 
> I reverted back to use GDK (from 
> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the events. 
> 
> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
> 
> There's also some cleaup.
> 
> To do general testing (two tests were added):
> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`
> 
> Information for reviewing:
> * Previously an offscreen window where used to pass events. Now it gets the 
> window were Drag initially started  
> (`WindowContextBase::sm_mouse_drag_window`);
> * There's a new `DragSourceContext` instead of global variables;
> * DragView were simplified;
> * It now uses a event hook when drag starts to listen to Drag/DragView events;
> * It handles `GDK_GRAB_BROKEN` events (I still need to figure it out a test 
> case for this - It should happen when another window grabs the device during 
> the drag);
> * It uses `gdk_threads_add_idle_full` to end drag so the `GDK_BUTTON_RELEASE` 
> event will continue before ending the drag. This is needed because 
> `WindowContext` will notify java about the button release;
> * It also notifies "Drag Dropped" on DND Source if the drop is on the same 
> app, because it needs to do so before mouse button is released - this method 
> is more straightforward;
> * The Scenario were the drag source window closes during the drag is now 
> covered;

Thiago Milczarek Sayao has updated the pull request incrementally with one 
additional commit since the last revision:

  Rework 2

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/986/files
  - new: https://git.openjdk.org/jfx/pull/986/files/7fe6ff40..04c4c954

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=19
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=18-19

  Stats: 88 lines in 3 files changed: 42 ins; 24 del; 22 mod
  Patch: https://git.openjdk.org/jfx/pull/986.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/986/head:pull/986

PR: https://git.openjdk.org/jfx/pull/986

Re: RFR: 8299423: JavaFX Mac system menubar leaks [v5]

2023-01-16 Thread Michael Hall 
Not directly related but I recently was looking at the Mac system menubar for 
javaFX applications noticing that they do not get an ‘About’ menu item in the 
Application menu.

I filed a bug that was just accepted today.

JDK-8300180 : [macos] Default About menu item not shown for javaFX applications
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8300180 


It involves changes to some of the legacy Apple code (ApplicationDelegate.m) 
and changes to the javaFX application to initialize this code and also to use 
java.awt.Desktop.
I was able to get javaFX applications an ‘About’ menu item by default, as all 
Mac applications including jpackage Swing ones do. 
Also with additional changes I was able to use java.awt.Desktop to get a custom 
aboutHandler for this menu item for applications that want to provide their own.

I had tried doing this with just javaFX by simply getting a menubar and setting 
it to system, but this didn’t work. Possibly there is more to this that I 
should of done?
I might look a little at what javaFX has for the Mac system menubar.

Re: RFR: 8290310: ChangeListener events are incorrect or misleading when a nested change occurs [v2]

2023-01-16 Thread Michael Strauß 
On Tue, 3 Jan 2023 09:42:05 GMT, John Hendrikx  wrote:

>> This contains the following:
>> - Nested changes or invalidations using ExpressionHelper are delayed until 
>> the current emission completes
>>   - This fixes odd change events being produced (with incorrect oldValue)
>>   - Also fixes a bug in ExpressionHelper where a nested change would unlock 
>> the listener list early, which could cause a 
>> `ConcurrentModificationException` if a nested change was combined with a 
>> remove/add listener call
>> - A test for ExpressionHelper to verify the new behavior
>> - A test for all *Property and *Binding classes that verifies correct 
>> listener behavior at the API level (this tests gets 85% coverage on 
>> ExpressionHelper on its own, the only thing it is not testing is the locking 
>> behavior, which is not relevant at the API level).
>> - A fix for `WebColorFieldSkin` which triggered a nested change which used a 
>> flag to prevent an event loop (I've changed it now to match how 
>> `DoubleFieldSkin` and `IntegerFieldSkin` do it
>
> John Hendrikx has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains two commits:
> 
>  - Merge branch 'master' of https://git.openjdk.org/jfx into 
> feature/delayed-nested-emission
>  - Delay nested event emission

Marked as reviewed by mstrauss (Committer).

-

PR: https://git.openjdk.org/jfx/pull/837

Re: RFR: 8299423: JavaFX Mac system menubar leaks [v5]

2023-01-16 Thread Michael Strauß 
On Fri, 13 Jan 2023 09:18:56 GMT, Florian Kirmaier  
wrote:

>> This PR fixes the leak in the mac system menu bar.
>> 
>> Inside the native code, NewGlobalRef is called for the callable.
>> Which makes it into a "GC-Root" until DeleteGlobalRef is called.
>> 
>> The DeleteGlobalRef is never called for the MenuEntry, if it's removed from 
>> the menu without removing it's callable.
>> This PR adds logic, whether the Menu is inserted. If it's not inserted in a 
>> Menu anymore, then DeleteGlobalRef is called, by calling `_setCallback` with 
>> the callable "null".
>> 
>> The unit test verifies, that this bug happened without this change, but no 
>> longer happens with this change.
>
> Florian Kirmaier has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8299423
>   we now use the junit5 api

Marked as reviewed by mstrauss (Committer).

-

PR: https://git.openjdk.org/jfx/pull/987

Re: RFR: 8277848 Binding and Unbinding to List leads to memory leak [v8]

2023-01-16 Thread Michael Strauß 
On Thu, 12 Jan 2023 08:53:14 GMT, Florian Kirmaier  
wrote:

>> Making the initial listener of the ListProperty weak fixes the problem.
>> The same is fixed for Set and Map.
>> Due to a smart implementation, this is done without any performance drawback.
>> (The trick is to have an object, which is both the WeakReference and the 
>> Changelistener)
>> By implying the same trick to the InvalidationListener, this should even 
>> improve the performance of the collection properties.
>
> Florian Kirmaier has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge remote-tracking branch 'origjfx/master' into 
> JDK-8277848-list-binding-leak
>
># Conflicts:
>#  
> modules/javafx.base/src/main/java/javafx/beans/property/ListPropertyBase.java
>#  
> modules/javafx.base/src/main/java/javafx/beans/property/SetPropertyBase.java
>#  
> modules/javafx.base/src/test/java/test/javafx/beans/property/SetPropertyBaseTest.java
>  - Merge remote-tracking branch 'origjfx/master' into 
> JDK-8277848-list-binding-leak
>  - JDK-8277848
>Added tests to ensure no premature garbage collection is happening.
>  - JDK-8277848
>Added 3 more tests to verify that a bug discussed in the PR does not 
> appear.
>  - JDK-8277848
>Added the 3 requests whitespaces
>  - JDK-8277848
>Further optimization based code review.
>This Bugfix should now event improve the performance
>  - JDK-8277848
>Added missing change
>  - JDK-8277848
>Fixed memoryleak, when binding and unbinding a ListProperty. The same was 
> fixed for SetProperty and MapProperty.

Marked as reviewed by mstrauss (Committer).

-

PR: https://git.openjdk.org/jfx/pull/689

Re: Style themes API

2023-01-16 Thread Michael Strauß 
Hi Pedro,
see my comments below.


On Mon, Jan 16, 2023 at 1:10 PM Pedro Duque Vieira
 wrote:
>
> No problem, I mentioned those in my first comment but probably got lost in 
> this conversation.
> 2 – I think the ability to specify in the StyleTheme whether it is a user 
> agent stylesheet, or an author stylesheet could be of interest. Most of the 
> themes I know are composed of author stylesheets (they use the 
> getStylesheets() API in Scene) so migration to using the StyleTheme API would 
> be easier if one could specify this. Conversely specifying that the 
> StyleTheme is a user agent stylesheet will also be very useful in the cases 
> where we’re creating a theme but don’t want it to override styles specified 
> through code, etc.
>
> I'll now also add that you might want to create a theme, from the get go, 
> that doesn't get trumped by any rule set through code, either code itself on 
> in FXML files (which will be code anyway). So that the theme css is what 
> always drives the presentation of your app (it has priority).
> This is already possible by choosing to add your stylesheets to the Scene 
> getStylesheets() method or by using the setUserAgentStylesheet() method.

Currently, the order of precedence is as follows (from highest to lowest):
1. Node.style
2. Parent.stylesheets
3. Scene.stylesheets
4. StyleableProperty values set from code
5. Application.styleTheme
6. Application.userAgentStylesheet

I understand you're suggesting that a style theme could effectively be
inserted between 3 and 4, such that it overrides local values set from
code, but not values set in Scene stylesheets.
I'm not sure that the added complexity is worth the marginal benefits.
As far as JavaFX has always had the concept of built-in "themes",
users could reliably override any StyleableProperty value from code.
Developers might be surprised to find out that they can't set the
value of a StyleableProperty from code because an application-wide
style theme overrides any local changes.
The migration story from Scene stylesheets to a StyleTheme might be a
little bit easier, but it complicates the mental model of how themes
interact with the CSS cascade.



> Ok thanks for clarifying what happens on Mac. Yes on the Windows OS, JavaFX 
> won't honor your system wide settings. If your system is in dark mode the 
> frames will still be shown in light mode.
> But anyways, I think, the programmer should be able to toggle the dark/light 
> mode setting irrespective of what is defined in the system. There are many 
> apps where you, as a user, can choose dark or light mode in the app itself 
> and it will honor it regardless of what you have defined in the system (e.g. 
> Intellij, Slack, etc, etc). Some will allow you to set whether you want to 
> use the system setting for light/dark mode or your own setting in the app.
> This can be of value in cases where you, as a user, prefer to use an app in 
> dark mode and others in some other mode, so you can choose themes on an app 
> by app basis.
> Or perhaps the developer wants to design an app that's just dark. In that 
> case you'd want the ability to toggle the frame decorations to being dark 
> (all the time).

I agree that it would be nice to have a way to programmatically
control the appearance of system-provided window decorations.
It's not entirely related to style themes: a style theme is an
application-wide concept, while window decorations are specific to
each individual window.
Maybe we could do this similarly to the Stage.initStyle method, which
serves a similar purpose of setting up window decorations.



> Just to be clear I was just talking about how we would present this API 
> (platform preferences). This property would be a way to more easily know if 
> the system is in dark/light mode, the accent color, without needing to know 
> the specific "key".

If we wanted to expose this front-and-center, we could add these
properties to the Platform.Properties interface:

public interface Preferences extends ObservableMap {
ReadOnlyBooleanProperty darkModeProperty();
ReadOnlyObjectProperty accentColorProperty();
...
}

[jfx19] RFR: 8300226: Change JavaFX release version to 19.0.2

2023-01-16 Thread Johan Vos 
Bump JavaFX release version to 19.0.2

-

Commit messages:
 - Bump JavaFX release version to 19.0.2

Changes: https://git.openjdk.org/jfx/pull/997/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=997&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8300226
  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jfx/pull/997.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/997/head:pull/997

PR: https://git.openjdk.org/jfx/pull/997

Re: RFR: 8273379 - GTK3 stops sending key events during drag and drop [v19]

2023-01-16 Thread Thiago Milczarek Sayao 
> This PR fixes 8273379.
> 
> I reverted back to use GDK (from 
> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the events. 
> 
> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
> 
> There's also some cleaup.
> 
> To do general testing (two tests were added):
> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`
> 
> Information for reviewing:
> * Previously an offscreen window where used to pass events. Now it gets the 
> window were Drag initially started  
> (`WindowContextBase::sm_mouse_drag_window`);
> * There's a new `DragSourceContext` instead of global variables;
> * DragView were simplified;
> * It now uses a event hook when drag starts to listen to Drag/DragView events;
> * It handles `GDK_GRAB_BROKEN` events (I still need to figure it out a test 
> case for this - It should happen when another window grabs the device during 
> the drag);
> * It uses `gdk_threads_add_idle_full` to end drag so the `GDK_BUTTON_RELEASE` 
> event will continue before ending the drag. This is needed because 
> `WindowContext` will notify java about the button release;
> * It also notifies "Drag Dropped" on DND Source if the drop is on the same 
> app, because it needs to do so before mouse button is released - this method 
> is more straightforward;
> * The Scenario were the drag source window closes during the drag is now 
> covered;

Thiago Milczarek Sayao has updated the pull request incrementally with two 
additional commits since the last revision:

 - Fix drag leave
 - Fix cursor + improve drag move

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/986/files
  - new: https://git.openjdk.org/jfx/pull/986/files/a036ce6f..7fe6ff40

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=18
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=17-18

  Stats: 36 lines in 1 file changed: 17 ins; 9 del; 10 mod
  Patch: https://git.openjdk.org/jfx/pull/986.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/986/head:pull/986

PR: https://git.openjdk.org/jfx/pull/986

Re: RFR: 8273379 - GTK3 stops sending key events during drag and drop [v18]

2023-01-16 Thread Thiago Milczarek Sayao 
> This PR fixes 8273379.
> 
> I reverted back to use GDK (from 
> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the events. 
> 
> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
> 
> There's also some cleaup.
> 
> To do general testing (two tests were added):
> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`
> 
> Information for reviewing:
> * Previously an offscreen window where used to pass events. Now it gets the 
> window were Drag initially started  
> (`WindowContextBase::sm_mouse_drag_window`);
> * There's a new `DragSourceContext` instead of global variables;
> * DragView were simplified;
> * It now uses a event hook when drag starts to listen to Drag/DragView events;
> * It handles `GDK_GRAB_BROKEN` events (I still need to figure it out a test 
> case for this - It should happen when another window grabs the device during 
> the drag);
> * It uses `gdk_threads_add_idle_full` to end drag so the `GDK_BUTTON_RELEASE` 
> event will continue before ending the drag. This is needed because 
> `WindowContext` will notify java about the button release;
> * It also notifies "Drag Dropped" on DND Source if the drop is on the same 
> app, because it needs to do so before mouse button is released - this method 
> is more straightforward;
> * The Scenario were the drag source window closes during the drag is now 
> covered;

Thiago Milczarek Sayao has updated the pull request incrementally with one 
additional commit since the last revision:

  Fix icon size on dragview

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/986/files
  - new: https://git.openjdk.org/jfx/pull/986/files/b130e740..a036ce6f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=17
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=16-17

  Stats: 4 lines in 1 file changed: 1 ins; 2 del; 1 mod
  Patch: https://git.openjdk.org/jfx/pull/986.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/986/head:pull/986

PR: https://git.openjdk.org/jfx/pull/986

Re: RFR: 8273379 - GTK3 stops sending key events during drag and drop [v17]

2023-01-16 Thread Thiago Milczarek Sayao 
> This PR fixes 8273379.
> 
> I reverted back to use GDK (from 
> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the events. 
> 
> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
> 
> There's also some cleaup.
> 
> To do general testing (two tests were added):
> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`
> 
> Information for reviewing:
> * Previously an offscreen window where used to pass events. Now it gets the 
> window were Drag initially started  
> (`WindowContextBase::sm_mouse_drag_window`);
> * There's a new `DragSourceContext` instead of global variables;
> * DragView were simplified;
> * It now uses a event hook when drag starts to listen to Drag/DragView events;
> * It handles `GDK_GRAB_BROKEN` events (I still need to figure it out a test 
> case for this - It should happen when another window grabs the device during 
> the drag);
> * It uses `gdk_threads_add_idle_full` to end drag so the `GDK_BUTTON_RELEASE` 
> event will continue before ending the drag. This is needed because 
> `WindowContext` will notify java about the button release;
> * It also notifies "Drag Dropped" on DND Source if the drop is on the same 
> app, because it needs to do so before mouse button is released - this method 
> is more straightforward;
> * The Scenario were the drag source window closes during the drag is now 
> covered;

Thiago Milczarek Sayao has updated the pull request incrementally with one 
additional commit since the last revision:

  Stop DRAG on GDK_DELETE

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/986/files
  - new: https://git.openjdk.org/jfx/pull/986/files/daade445..b130e740

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=16
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=15-16

  Stats: 15 lines in 3 files changed: 10 ins; 3 del; 2 mod
  Patch: https://git.openjdk.org/jfx/pull/986.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/986/head:pull/986

PR: https://git.openjdk.org/jfx/pull/986

Re: RFR: 8273379 - GTK3 stops sending key events during drag and drop [v16]

2023-01-16 Thread Thiago Milczarek Sayao 
> This PR fixes 8273379.
> 
> I reverted back to use GDK (from 
> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the events. 
> 
> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
> 
> There's also some cleaup.
> 
> The most changed part is that I had to move `process_events` to 
> `glass_evloop`  because it's reused in glass_dnd.
> 
> To do general testing:
> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`

Thiago Milczarek Sayao has updated the pull request incrementally with one 
additional commit since the last revision:

  It works!

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/986/files
  - new: https://git.openjdk.org/jfx/pull/986/files/f916a108..daade445

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=15
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=14-15

  Stats: 6 lines in 1 file changed: 0 ins; 1 del; 5 mod
  Patch: https://git.openjdk.org/jfx/pull/986.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/986/head:pull/986

PR: https://git.openjdk.org/jfx/pull/986

Re: RFR: 8273379 - GTK3 stops sending key events during drag and drop [v15]

2023-01-16 Thread Thiago Milczarek Sayao 
> This PR fixes 8273379.
> 
> I reverted back to use GDK (from 
> [8225571](https://bugs.openjdk.org/browse/JDK-8225571)) to handle the events. 
> 
> It may also fix [8280383](https://bugs.openjdk.org/browse/JDK-8280383).
> 
> There's also some cleaup.
> 
> The most changed part is that I had to move `process_events` to 
> `glass_evloop`  because it's reused in glass_dnd.
> 
> To do general testing:
> `java @build/run.args -jar apps/toys/DragDrop/dist/DragDrop.jar`

Thiago Milczarek Sayao has updated the pull request with a new target base due 
to a merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains 47 additional commits 
since the last revision:

 - Rework
 - Rework events
 - Merge branch 'master' into 8273379-dnd-keys
 - Merge branch 'openjdk:master' into master
 - Device Grab Work
 - Adjustments
 - Replace g_warning
 - Improvements
 - Add test
 - Add testing
 - ... and 37 more: https://git.openjdk.org/jfx/compare/89f8ac54...f916a108

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/986/files
  - new: https://git.openjdk.org/jfx/pull/986/files/08d05af8..f916a108

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=14
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=986&range=13-14

  Stats: 4806 lines in 81 files changed: 3541 ins; 825 del; 440 mod
  Patch: https://git.openjdk.org/jfx/pull/986.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/986/head:pull/986

PR: https://git.openjdk.org/jfx/pull/986

Re: Style themes API

2023-01-16 Thread Pedro Duque Vieira 
Hi again Michael,

Thanks for taking the time to answer my comments.
As always, it's easier to comment on and criticize than to do the actual
work of implementing the feature. So thanks again for the patience.

My comments below:


I don't quite understand this. Can you elaborate on what the
> advantages would be, and how (and for what purpose) users of the API
> would use such a flag?


No problem, I mentioned those in my first comment but probably got lost in
this conversation.
2 – I think the ability to specify in the StyleTheme whether it is a user
agent stylesheet, or an author stylesheet could be of interest. Most of the
themes I know are composed of author stylesheets (they use the
getStylesheets() API in Scene) so migration to using the StyleTheme API
would be easier if one could specify this. Conversely specifying that the
StyleTheme is a user agent stylesheet will also be very useful in the cases
where we’re creating a theme but don’t want it to override styles specified
through code, etc.

I'll now also add that you might want to create a theme, from the get go,
that doesn't get trumped by any rule set through code, either code itself
on in FXML files (which will be code anyway). So that the theme css is what
always drives the presentation of your app (it has priority).
This is already possible by choosing to add your stylesheets to the Scene
getStylesheets() method or by using the setUserAgentStylesheet() method.


I don't think that we need any new API for dark mode window frames.
> On macOS, the color of the window frame is already adjusted to match
> the system-wide dark/light mode.
> On Windows, this is not the case:


https://learn.microsoft.com/en-us/windows/apps/desktop/modernize/apply-windows-themes


"Not all Win32 applications support Dark mode, so
> Windows gives Win32 apps a light title bar by
> default. If you are prepared to support Dark mode,
> you can ask Windows to draw the dark title bar
> instead when Dark mode is enabled."


Since JavaFX already uses dark frames on macOS, why not do the same on
> Windows?
> We can simply enable this behavior in general, and provide a system
> property to revert to the old behavior of always using light frames,
> even if dark mode is enabled.


Ok thanks for clarifying what happens on Mac. Yes on the Windows OS, JavaFX
won't honor your system wide settings. If your system is in dark mode the
frames will still be shown in light mode.
But anyways, I think, the programmer should be able to toggle the
dark/light mode setting irrespective of what is defined in the system.
There are many apps where you, as a user, can choose dark or light mode in
the app itself and it will honor it regardless of what you have defined in
the system (e.g. Intellij, Slack, etc, etc). Some will allow you to set
whether you want to use the system setting for light/dark mode or your own
setting in the app.
This can be of value in cases where you, as a user, prefer to use an app in
dark mode and others in some other mode, so you can choose themes on an app
by app basis.
Or perhaps the developer wants to design an app that's just dark. In that
case you'd want the ability to toggle the frame decorations to being dark
(all the time).


Here's a simple implementation that exposes dark mode and accent
> coloring as JavaFX properties on a theme base class:
> https://gist.github.com/mstr2/ba2f9cba659953788008fed1e9b2a031


We might consider adding something like this to JavaFX in the future,
> but for the time being, I'd rather let third-party libraries explore
> higher-level APIs first.


Just to be clear I was just talking about how we would present this API
(platform preferences). This property would be a way to more easily know if
the system is in dark/light mode, the accent color, without needing to know
the specific "key".

Thanks again!

On Mon, Jan 16, 2023 at 4:01 AM Michael Strauß 
wrote:

> My comments below:
>
> On Sun, Jan 15, 2023 at 1:25 PM Pedro Duque Vieira
>  wrote:
> >
> > I think we could add that it has higher precedence than a user agent
> stylesheets but that's it's not an author stylesheet. We can also possibly
> briefly say what a user agent stylesheet and author stylesheet are or
> perhaps better yet, point to the css reference doc section that states it?
>
> Sure, I'll add some additional documentation.
>
>
>
> > Yes, I think minimizing changes to the CSS subsystem is a good idea.
> > I didn't mean we should make changes to the CSS subsystem, I
> meant/proposed to add a flag or property in StyleThemes (the new API) that
> programmers would then set in their StyleThemes and would define whether
> the StyleTheme he's creating is a user agent stylesheet or an author
> stylesheet.
>
> I don't quite understand this. Can you elaborate on what the
> advantages would be, and how (and for what purpose) users of the API
> would use such a flag?
>
>
>
> > I disagree here. I think this would be too low level for third party
> apps to add suppor

Re: [jfx20] RFR: 8290863: Update the documentation of Virtualized controls to include the best practice of not using Nodes directly in the item list [v2]

2023-01-16 Thread Ajit Ghaisas 
> This PR adds a warning about inserting Nodes directly into the virtualized 
> containers such as ListView, TreeView, TableView and TreeTableView. It also 
> adds code snippets showing the recommended pattern of using a custom cell 
> factory for each of the virtualized control.

Ajit Ghaisas has updated the pull request incrementally with one additional 
commit since the last revision:

  Review fixes

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/995/files
  - new: https://git.openjdk.org/jfx/pull/995/files/4325a8e4..e46e31b4

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=995&range=01
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=995&range=00-01

  Stats: 116 lines in 4 files changed: 9 ins; 74 del; 33 mod
  Patch: https://git.openjdk.org/jfx/pull/995.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/995/head:pull/995

PR: https://git.openjdk.org/jfx/pull/995

RFR: 8230833: LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY

2023-01-16 Thread Karthik P K 
Ignore text condition was not considered in `computePrefHeight` method while 
calculating the Label height.

Added `isIgnoreText` condition in `computePrefHeight` method while calculating 
Label height.

Tests are present for all the ContentDisplay types. Modified tests which were 
failing because of the new height value getting calculated.

-

Commit messages:
 - Label height issue fix

Changes: https://git.openjdk.org/jfx/pull/996/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=996&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8230833
  Stats: 31 lines in 2 files changed: 6 ins; 8 del; 17 mod
  Patch: https://git.openjdk.org/jfx/pull/996.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/996/head:pull/996

PR: https://git.openjdk.org/jfx/pull/996