Re: RFR: WIP: 8260528: Clean glass-gtk sizing and positioning code [v22]

2022-11-04 Thread Thiago Milczarek Sayao
> This cleans size and positioning code, reducing special cases, code > complexity and size. > > Changes: > > - cached extents: 28, 1, 1, 1 are old defaults - modern gnome uses different > sizes. It does not assume any size because it varies - it does cache because > it's unlikely to vary on

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v11]

2022-11-04 Thread Andy Goryachev
> Setting a null selection model in TableView and TreeTableView produce NPE on > sorting (and probably in some other situations) because the check for null is > missing in several places. > > Setting a null selection model is a valid way to disable selection in a > (tree)table. > > There is

Re: RFR: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin

2022-11-04 Thread Kevin Rushforth
On Tue, 27 Sep 2022 19:36:46 GMT, Andy Goryachev wrote: > Using new Skin.install() method to properly set and remove > inputMethodRequests property in TextInputControl. > > This avoids memory leaks resulting from skin change, as well as honors > user-set properties when installing an

Re: RFR: 8296409: Multiple copies of accelerator change listeners are added to MenuItems, but only 1 is removed

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 16:07:54 GMT, Dean Wookey wrote: > When menu buttons are added and removed from the scene, an accelerator change > listener is added to each menu item in the menu. There is nothing stopping > the same change listener being added multiple times. > > MenuButtonSkinBase calls

Re: RFR: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin

2022-11-04 Thread Andy Goryachev
On Tue, 27 Sep 2022 19:36:46 GMT, Andy Goryachev wrote: > Using new Skin.install() method to properly set and remove > inputMethodRequests property in TextInputControl. > > This avoids memory leaks resulting from skin change, as well as honors > user-set properties when installing an

Re: RFR: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin

2022-11-04 Thread Kevin Rushforth
On Fri, 4 Nov 2022 20:36:00 GMT, Andy Goryachev wrote: > > do you still want / need to move the initialization logic to the > > `Skin::install` method? > > good question. In the case of `setOnInputMethodTextChanged` -> > `control.addEventHandler(InputMethodEvent.INPUT_METHOD_TEXT_CHANGED` the

Re: RFR: 8296330: Tests: Add layout container snapping tests

2022-11-04 Thread Andy Goryachev
On Thu, 3 Nov 2022 21:15:08 GMT, Marius Hanl wrote: > While checking https://bugs.openjdk.org/browse/JDK-8295078 I found much more > layout container which do not snap their children correctly. > > The goal of this PR is to add snapping tests for all layout container. > After that we can

Re: RFR: 8230231: font-family not updated in HTMLEditor [v3]

2022-11-04 Thread Kevin Rushforth
On Wed, 21 Jul 2021 14:56:11 GMT, Hadzic Samir wrote: >> Fix for https://github.com/javafxports/openjdk-jfx/issues/573 >> >> Issue on JBS bug tracking : https://bugs.openjdk.java.net/browse/JDK-8230231 >> >> Fix: Check for quote when updating the font-family comboBox. >> >> A new font is

Re: RFR: 8295324: JavaFX: Blank pages when printing [v3]

2022-11-04 Thread Kevin Rushforth
On Fri, 28 Oct 2022 13:54:40 GMT, eduardsdv wrote: >> This fixes a race condition between application and 'Print Job Thread' >> threads when printing. >> >> The race condition occurs when application thread calls `endJob()`, which in >> effect sets the `jobDone` flag to true, >> and when the

Re: RFR: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 20:19:52 GMT, Kevin Rushforth wrote: > do you still want / need to move the initialization logic to the > `Skin::install` method? good question. In the case of `setOnInputMethodTextChanged` -> `control.addEventHandler(InputMethodEvent.INPUT_METHOD_TEXT_CHANGED` the code

Re: RFR: 8294809: ListenerHelper for managing and disconnecting listeners [v9]

2022-11-04 Thread Andy Goryachev
> Introduction > > There is a number of places where various listeners (strong as well as weak) > are added, to be later disconnected in one go. For example, Skin > implementations use dispose() method to clean up the listeners installed in > the corresponding Control (sometimes using >

Re: RFR: 8294809: ListenerHelper for managing and disconnecting listeners [v8]

2022-11-04 Thread Andy Goryachev
On Mon, 31 Oct 2022 16:45:51 GMT, Andy Goryachev wrote: >> Introduction >> >> There is a number of places where various listeners (strong as well as weak) >> are added, to be later disconnected in one go. For example, Skin >> implementations use dispose() method to clean up the listeners

Re: RFR: 8294809: ListenerHelper for managing and disconnecting listeners [v8]

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 17:58:16 GMT, Kevin Rushforth wrote: >> Andy Goryachev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8294809: whitespace > > modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/IDisconnectable.java >

Re: RFR: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin

2022-11-04 Thread Kevin Rushforth
On Tue, 27 Sep 2022 19:36:46 GMT, Andy Goryachev wrote: > Using new Skin.install() method to properly install and uninstall > inputMethodTextChanged and inputMethodRequests properties on TextInputControl. > > This avoids memory leaks resulting from skin change, as well as honors > user-set

Re: RFR: 8295078: TextField blurry when inside an TitledPane -> AnchorPane [v3]

2022-11-04 Thread Kevin Rushforth
On Thu, 3 Nov 2022 21:21:42 GMT, Marius Hanl wrote: >> The problem here is, that the `AnchorPane` does not use its snapped insets. >> Therefore, the fix is to replace all `getInsets().getXXX` calls with their >> corresponding `snappedXXXInset()` methods. >> >> Note: The reason the `AnchorPane`

Re: RFR: 8296330: Tests: Add layout container snapping tests

2022-11-04 Thread Kevin Rushforth
On Thu, 3 Nov 2022 21:15:08 GMT, Marius Hanl wrote: > While checking https://bugs.openjdk.org/browse/JDK-8295078 I found much more > layout container which do not snap their children correctly. > > The goal of this PR is to add snapping tests for all layout container. > After that we can

Re: RFR: 8296409: Multiple copies of accelerator change listeners are added to MenuItems, but only 1 is removed

2022-11-04 Thread Kevin Rushforth
On Fri, 4 Nov 2022 16:07:54 GMT, Dean Wookey wrote: > When menu buttons are added and removed from the scene, an accelerator change > listener is added to each menu item in the menu. There is nothing stopping > the same change listener being added multiple times. > > MenuButtonSkinBase calls

Re: RFR: 8254676: Alert disables Tab selection when TabDragPolicy REORDER is used

2022-11-04 Thread Kevin Rushforth
On Mon, 19 Sep 2022 11:43:41 GMT, Ambarish Rapte wrote: >> Don't set TabHeader to mouseTransparent, since it might get stuck in that >> state (e.g. in case an Alert is shown). >> The TabPaneSkin deals with the dragging internally, and does not require the >> dragged node to be

Re: RFR: 8294809: ListenerHelper for managing and disconnecting listeners [v8]

2022-11-04 Thread Kevin Rushforth
On Mon, 31 Oct 2022 16:45:51 GMT, Andy Goryachev wrote: >> Introduction >> >> There is a number of places where various listeners (strong as well as weak) >> are added, to be later disconnected in one go. For example, Skin >> implementations use dispose() method to clean up the listeners

Re: RFR: 8296409: Multiple copies of accelerator change listeners are added to MenuItems, but only 1 is removed

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 16:07:54 GMT, Dean Wookey wrote: > When menu buttons are added and removed from the scene, an accelerator change > listener is added to each menu item in the menu. There is nothing stopping > the same change listener being added multiple times. > > MenuButtonSkinBase calls

Re: CFV: New OpenJFX Committer: Andy Goryachev

2022-11-04 Thread Thiago Milczarek Sayão
Vote: YES Em qui., 3 de nov. de 2022 às 14:06, Kevin Rushforth < kevin.rushfo...@oracle.com> escreveu: > I hereby nominate Andy Goryachev [1] to OpenJFX Committer. > > Andy is a member of the JavaFX team at Oracle who has contributed 22 > commits [2] to OpenJFX. > > Votes are due by November 17,

Re: RFR: 8294809: ListenerHelper for managing and disconnecting listeners [v8]

2022-11-04 Thread Kevin Rushforth
On Mon, 31 Oct 2022 16:45:51 GMT, Andy Goryachev wrote: >> Introduction >> >> There is a number of places where various listeners (strong as well as weak) >> are added, to be later disconnected in one go. For example, Skin >> implementations use dispose() method to clean up the listeners

Re: RFR: 8222210: JFXPanel popups open at wrong coordinates when using multiple hidpi monitors [v3]

2022-11-04 Thread Kevin Rushforth
On Wed, 2 Nov 2022 20:07:22 GMT, Johan Vos wrote: >> I suspect Math.floor() would be incorrect - we should use Math.round() for >> coordinates and Math.floor() for sizes (sizes will always be positive, I >> hope, and the rounded size will be slightly smaller than the original value). >> >>

Re: RFR: 8222210: JFXPanel popups open at wrong coordinates when using multiple hidpi monitors [v3]

2022-11-04 Thread Kevin Rushforth
On Wed, 2 Nov 2022 19:58:15 GMT, Johan Vos wrote: >> The root problem is actually broader than stated in the JBS issue. This PR >> now translates screencoordinates from absolute coordinates into coordinates >> that take the platformScale into account. >> The whole process is complicated by

Re: RFR: 8222210: JFXPanel popups open at wrong coordinates when using multiple hidpi monitors [v2]

2022-11-04 Thread Kevin Rushforth
On Wed, 2 Nov 2022 19:54:00 GMT, Johan Vos wrote: >> modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 400: >> >>> 398: float py = screen.getPlatformY(); >>> 399: newx = sx + (wx - px) * awtScaleX / pScaleX; >>> 400: newy = sy + (wy -

Re: RFR: 8296409: Multiple copies of accelerator change listeners are added to MenuItems, but only 1 is removed

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 16:07:54 GMT, Dean Wookey wrote: > When menu buttons are added and removed from the scene, an accelerator change > listener is added to each menu item in the menu. There is nothing stopping > the same change listener being added multiple times. > > MenuButtonSkinBase calls

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v10]

2022-11-04 Thread Andy Goryachev
> Setting a null selection model in TableView and TreeTableView produce NPE on > sorting (and probably in some other situations) because the check for null is > missing in several places. > > Setting a null selection model is a valid way to disable selection in a > (tree)table. > > There is

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v9]

2022-11-04 Thread Andy Goryachev
> Setting a null selection model in TableView and TreeTableView produce NPE on > sorting (and probably in some other situations) because the check for null is > missing in several places. > > Setting a null selection model is a valid way to disable selection in a > (tree)table. > > There is

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v8]

2022-11-04 Thread Andy Goryachev
> Setting a null selection model in TableView and TreeTableView produce NPE on > sorting (and probably in some other situations) because the check for null is > missing in several places. > > Setting a null selection model is a valid way to disable selection in a > (tree)table. > > There is

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v7]

2022-11-04 Thread Andy Goryachev
> Setting a null selection model in TableView and TreeTableView produce NPE on > sorting (and probably in some other situations) because the check for null is > missing in several places. > > Setting a null selection model is a valid way to disable selection in a > (tree)table. > > There is

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v5]

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 14:15:43 GMT, Jeanette Winzenburg wrote: >> Any comments on this? > > hmm .. maybe this is a mis-understanding: > > - the case selected_items looks fishy (also in the original) - seems to not > return if the selection is empty or now if the selection is null > - every known

Re: Switching to ANGLE ?

2022-11-04 Thread Kevin Rushforth
We briefly considered this several years ago. We are unlikely to switch Prism rendering to Angle. Having a dependency on another large third-party library in our core renderer is not a direction we plan to go. As a note, WebGL wouldn't fall out for free from this anyway, since the way WebKit's

RFR: 8296409: Multiple copies of accelerator change listeners are added to MenuItems, but only 1 is removed

2022-11-04 Thread Dean Wookey
When menu buttons are added and removed from the scene, an accelerator change listener is added to each menu item in the menu. There is nothing stopping the same change listener being added multiple times. MenuButtonSkinBase calls the

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v5]

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 14:15:43 GMT, Jeanette Winzenburg wrote: >> Any comments on this? > > hmm .. maybe this is a mis-understanding: > > - the case selected_items looks fishy (also in the original) - seems to not > return if the selection is empty or now if the selection is null > - every known

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v5]

2022-11-04 Thread Andy Goryachev
On Fri, 4 Nov 2022 11:45:06 GMT, Kevin Rushforth wrote: >> same comment, there is a fall through. > > No, there isn't. this is embarrassing - you are right. the only fall through case is on line 169, but it's clearly wrong, as it should not be falling through to CELL_AT_ROW_COLUMN. in the

Re: Switching to ANGLE ?

2022-11-04 Thread Anirvan Sarkar
There is an ANGLE RFE [1]. It is closed as a duplicate of an older WebGL RFE [2]. [1] : https://bugs.openjdk.org/browse/JDK-8134841 [2] : https://bugs.openjdk.org/browse/JDK-8091035 On Wed, 2 Nov 2022 at 7:56 PM, Mike Hearn wrote: > On a topic unrelated to Conveyor, I saw the JavaFX roadmap

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v5]

2022-11-04 Thread Jeanette Winzenburg
On Fri, 4 Nov 2022 05:07:17 GMT, Ajit Ghaisas wrote: >> There is a `return null` on line 176. >> In that case, we will return from either line 173 or line 176. Hence, there >> is no fall through. > > Any comments on this? hmm .. maybe this is a mis-understanding: - the case selected_items

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v5]

2022-11-04 Thread Kevin Rushforth
On Thu, 3 Nov 2022 16:00:30 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java >> line 192: >> >>> 190: return null; >>> 191: } >>> 192: // fall through >> >> There is no "fall through" here