Re: RFR: 8313138: Scrollbar Keyboard enhancement [v7]
On Mon, 6 May 2024 23:14:11 GMT, Andy Goryachev wrote: >> Adding alt-ctrl-LEFT/RIGHT/UP/DOWN (option-command-LEFT/RIGHT/UP/DOWN) key >> bindings to >> >> - ListView >> - TreeView >> - TableView >> - TreeTableView >> >> to support keyboard-only horizontal and vertical scrolling. The main reason >> for the change is to improve accessibility. >> >> **NOTE: For controls in right-to-left orientation, the direction of >> horizontal scrolling is reversed.** >> >> As far as I can tell, these key combinations do not interfere with editing. >> >> The proposed solution can be further optimized by adding a public method to >> the VirtualFlow class, something like >> >> >> public void horizontalUnitScroll(boolean right); >> public void verticalUnitScroll(boolean down); >> >> >> Q: Does this change require a CSR to explain the change in the controls' >> behavior? We don't yet have the key bindings documented in >> /doc-files/behavior >> >> Note: >> Jenkins headful test passed on all mac configurations, failed on all linux >> configurations (master branch failed also, so it is test issue), while >> windows configuration is not yet available. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > vertical scrolling tests LGTM... - Marked as reviewed by arapte (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1393#pullrequestreview-2057040934
Re: RFR: 8305418: [Linux] Replace obsolete XIM as Input Method Editor [v20]
On Tue, 14 May 2024 10:59:10 GMT, Glavo wrote: > I will no longer commit to this change. I've been closely following this PR and sincerely hope it can be reopened; it is of great significance and has a far-reaching impact. Thank you for your efforts. I also respect your decision, but I strongly wish for this PR to be reconsidered. - PR Comment: https://git.openjdk.org/jfx/pull/1080#issuecomment-2111639052
Re: RFR: 8279140: ComboBox can lose selected value on item change via setAll [v2]
On Thu, 9 May 2024 09:38:31 GMT, Karthik P K wrote: >> The `ComboBox` value was not set to previously selected value in the item >> list change listener when `setAll` method is used to change the items. Fixed >> the issue by restoring the selection in this case. >> >> Added a unit test to validate the fix. >> [JDK-8279139](https://bugs.openjdk.org/browse/JDK-8279139) is also fixed >> with this change and added a unit test for the same. > > Karthik P K has updated the pull request incrementally with one additional > commit since the last revision: > > Fix JDK-8279139 Yes there are few bugs related to ComboBox. I think we can fix JDK-8090221 separately. The issue JDK-8279139, which is closed as duplicate of this issue is fixed, but changing the test slightly shows another issue which is mentioned in JDK-8263942. Since we have different scenarios like these, we can fix these issue with these PRs one by one. So I believe other issues can be fixed separately. - PR Comment: https://git.openjdk.org/jfx/pull/1452#issuecomment-2111637496
Re: RFR: 8273743: KeyCharacterCombination for "+" does not work on US QWERTY keyboard layout [v2]
On Tue, 14 May 2024 22:29:18 GMT, Martin Fox wrote: >> On Linux getKeyCodeForChar does not consult the current keyboard layout. For >> example, it assumes the “+” character is on KeyCode.PLUS even on layouts >> which don’t generate KeyCode.PLUS. The result is that most >> KeyCharacterCombinations don’t work. >> >> This PR fixes this using the same approach that Mac glass uses. When the >> user types a key we lookup all the characters that key might generate and >> put them in a map. getKeyCodeForChar then consults the map to find the Java >> key code. This ensures that we only pay attention to keys that are on the >> user’s physical keyboard. >> >> (Some Linux layout maps are expansive and include entries for keys that may >> be on the user’s keyboard but probably aren’t, like “(“ and “)” on the >> numeric keypad. If we simply ask for all entries that generate a given >> character we can get multiple hits with no good way to determine which ones >> are physically present.) >> >> This PR also contains fixes to the Robot to enable testing. When Glass >> consults the GdkKeymap to determine which keys might generate a keyval GDK >> returns a list covering every installed layout and shift level. The old >> Glass code simply picked the first entry in the list. This PR iterates >> through the list looking for one that works for the current layout and >> correct shift level. > > Martin Fox 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 ten additional commits since > the last revision: > > - Merge remote-tracking branch 'upstream/master' into linuxcharcombo > - Comment fixes > - Consistency in naming conventions and comment cleanup. > - Merge remote-tracking branch 'upstream/master' into linuxcharcombo > - Expanded robot lookup table, general cleanup > - Merge remote-tracking branch 'upstream/master' into linuxcharcombo > - Resolving duplicates for Robot, fallback for getKeyCodeForChar > - Merge remote-tracking branch 'upstream/master' into linuxcharcombo > - KeyCharacterCombination fixes on Linux I've updated to the latest master. This will probably be the last time I refresh this pull request; I can only hold off the bots for so long. - PR Comment: https://git.openjdk.org/jfx/pull/1373#issuecomment-2111343296
Re: RFR: 8279140: ComboBox can lose selected value on item change via setAll [v2]
On Thu, 9 May 2024 09:38:31 GMT, Karthik P K wrote: >> The `ComboBox` value was not set to previously selected value in the item >> list change listener when `setAll` method is used to change the items. Fixed >> the issue by restoring the selection in this case. >> >> Added a unit test to validate the fix. >> [JDK-8279139](https://bugs.openjdk.org/browse/JDK-8279139) is also fixed >> with this change and added a unit test for the same. > > Karthik P K has updated the pull request incrementally with one additional > commit since the last revision: > > Fix JDK-8279139 It looks like we have quite a few bugs related to different aspects of ComboBox. This change is definitely an improvement, as it fixes the scenario described in JDK-8279140. This PR changes parts of the code that fixed JDK-8087838. While I don't see a regression in respect to JDK-8087838, I do see a different behavior with and without this fix in JDK-8090221, using the test app there (to be specific, with the fix in this PR, the tester in JDK-8090221 continues to display the selected item in the combo box even though the output shows `Selected Item After: null` Do you think that JDK-8090221 scenario is a different case and should be addressed separately? - PR Comment: https://git.openjdk.org/jfx/pull/1452#issuecomment-2111241863
Re: RFR: 8273743: KeyCharacterCombination for "+" does not work on US QWERTY keyboard layout [v2]
> On Linux getKeyCodeForChar does not consult the current keyboard layout. For > example, it assumes the “+” character is on KeyCode.PLUS even on layouts > which don’t generate KeyCode.PLUS. The result is that most > KeyCharacterCombinations don’t work. > > This PR fixes this using the same approach that Mac glass uses. When the user > types a key we lookup all the characters that key might generate and put them > in a map. getKeyCodeForChar then consults the map to find the Java key code. > This ensures that we only pay attention to keys that are on the user’s > physical keyboard. > > (Some Linux layout maps are expansive and include entries for keys that may > be on the user’s keyboard but probably aren’t, like “(“ and “)” on the > numeric keypad. If we simply ask for all entries that generate a given > character we can get multiple hits with no good way to determine which ones > are physically present.) > > This PR also contains fixes to the Robot to enable testing. When Glass > consults the GdkKeymap to determine which keys might generate a keyval GDK > returns a list covering every installed layout and shift level. The old Glass > code simply picked the first entry in the list. This PR iterates through the > list looking for one that works for the current layout and correct shift > level. Martin Fox 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 ten additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into linuxcharcombo - Comment fixes - Consistency in naming conventions and comment cleanup. - Merge remote-tracking branch 'upstream/master' into linuxcharcombo - Expanded robot lookup table, general cleanup - Merge remote-tracking branch 'upstream/master' into linuxcharcombo - Resolving duplicates for Robot, fallback for getKeyCodeForChar - Merge remote-tracking branch 'upstream/master' into linuxcharcombo - KeyCharacterCombination fixes on Linux - Changes: - all: https://git.openjdk.org/jfx/pull/1373/files - new: https://git.openjdk.org/jfx/pull/1373/files/6de7864f..9aba1b0f Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1373&range=01 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1373&range=00-01 Stats: 35228 lines in 1034 files changed: 19566 ins; 6880 del; 8782 mod Patch: https://git.openjdk.org/jfx/pull/1373.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1373/head:pull/1373 PR: https://git.openjdk.org/jfx/pull/1373
Re: RFR: 8305418: [Linux] Replace obsolete XIM as Input Method Editor [v20]
On Sun, 18 Feb 2024 23:10:12 GMT, Thiago Milczarek Sayao wrote: >> This replaces obsolete XIM and uses gtk api for IME. >> Gtk uses [ibus](https://github.com/ibus/ibus) >> >> Gtk3+ uses relative positioning (as Wayland does), so I've added a Relative >> positioning on `InputMethodRequest`. >> >> [Screencast from 17-09-2023 >> 21:59:04.webm](https://github.com/openjdk/jfx/assets/30704286/6c398e39-55a3-4420-86a2-beff07b549d3) > > Thiago Milczarek Sayao has updated the pull request with a new target base > due to a merge or a rebase. The pull request now contains 94 commits: > > - Merge branch 'master' into new_ime > - Add signals to avoid warnings > - Merge branch 'master' into new_ime > ># Conflicts: ># modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp > - Add check for jview > - - Fix comment path > - - Fix double keyrelease > - - Use a work-around to relative positioning (until wayland is not > officially supported) >- Unref pango attr list > - Merge branch 'master' into new_ime > - Account the case of !filtered > - Fix change of focus when on preedit + add filter on key release > - ... and 84 more: https://git.openjdk.org/jfx/compare/1fb56e33...eb988565 I do hope this PR is revived. I've never been able to configure a useable IME on Ubuntu with XIM enabled. I think sticking with XIM is tantamount to not having a functioning IME. - PR Comment: https://git.openjdk.org/jfx/pull/1080#issuecomment-2111045286
Re: RFR: 8331748: Update libxml2 to 2.12.6
On Tue, 14 May 2024 05:21:37 GMT, Hima Bindu Meda wrote: > Updated libxml to v2.12.6. Verified build and sanity testing on all > platforms. No issue seen Looks good. All testing is green. - Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1453#pullrequestreview-2056175249
Re: RFR: 8323511: Scrollbar Click jumps inconsistent amount of pixels [v6]
On Tue, 14 May 2024 14:44:58 GMT, Johan Vos wrote: > I'm not sure I understand the question I thought there were specific comments somewhere in the code. Never mind then. Thank you for responding! - PR Comment: https://git.openjdk.org/jfx/pull/1326#issuecomment-2110915337
Re: RFR: 8285893: Hiding dialog and showing new one causes dialog to be frozen
On Sat, 4 May 2024 22:55:13 GMT, Martin Fox wrote: > This PR is based on a discussion that happened over in PR #1324. Some of this > explanation is copied from that thread. > > When `exitNestedEventLoop` is called on the innermost loop the > invokeLaterDispatcher suspends operation until the loop finishes. But if you > immediately start a new event loop the previous one won't finish and the > dispatcher will jam up and stop dispatching indefinitely. > > When the invokeLaterDispatcher is told that the innermost loop is exiting it > sets `leavingNestedEventLoop` to true expecting it to be set to false when > the loop actually exits. When the dispatcher is told that a new event loop > has started it is not clearing `leavingNestedEventLoop` which is causing the > jam. Basically it should follow the same logic used in glass; leaving the > innermost loop updates a boolean indicating that the loop should exit but if > a new loop is started the boolean is set back to a running state since it now > applies to the new loop, not the previous one. > > I suspect the invokeLaterDispatcher exists in part to deal with the specifics > of how deferred runnables are handled on the Mac. I investigated this a bit > and wrote up some comments in the Mac glass code. I now understand what’s going wrong on Linux but I don’t know how to fix it. When the test case tries to leave the innermost loop (let’s call it A) the cross-platform Java code calls Application.leaveNestedEventLoop to tell glass that the loop should exit. But before the loop can exit the test case starts a new loop leading to a call to Application.enterNestedEventLoop. On Mac the leaveNestedEventLoop call sets a few global state variables in glass and the enterNestedEventLoop call wipes out all those changes. So as far as glass is concerned the leaveNestedEventLoop call was never made. This works because the cross-platform Java code is holding onto an object that represents loop A; it’s down one level in the event loop stack and marked as LEAVING. When that object reaches the top of the event loop stack the core will once again call Application.leaveNestedEventLoop to exit loop A. On Linux leaveNestedEventLoop makes changes in glass that cannot be erased or undone (it calls gtk_main_quit which updates internal GTK state that we don’t have access to). In the end GTK will exit the loops in the correct order but it will exit loop A before the core code has a chance to call Application.leaveNestedEventLoop again. This is throwing off the bookkeeping including some debug checks. (JavaFX struggles with this scenario because the original test case behaves in such an unexpected way. While processing a click event on a modal stage the event handler hides that modal and then immediately starts a new modal. The original modal can’t really go away; the handler for the click event originated there and is still in progress. That first modal might not be visible but it can’t truly close until the second modal is dismissed.) - PR Comment: https://git.openjdk.org/jfx/pull/1449#issuecomment-2110892622
Re: RFR: 8329011: Update SQLite to 3.45.3
On Tue, 14 May 2024 11:45:06 GMT, Hima Bindu Meda wrote: > Update SQLite to v3.45.3.Verified build on all platforms. Sanity testing > looks fine. Looks good. - Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1454#pullrequestreview-2056068196
Re: RFR: 8329011: Update SQLite to 3.45.3
On Tue, 14 May 2024 11:45:06 GMT, Hima Bindu Meda wrote: > Update SQLite to v3.45.3.Verified build on all platforms. Sanity testing > looks fine. Everything built fine. - Marked as reviewed by sykora (Author). PR Review: https://git.openjdk.org/jfx/pull/1454#pullrequestreview-2055796129
Re: RFR: 8323511: Scrollbar Click jumps inconsistent amount of pixels [v6]
On Thu, 9 May 2024 17:45:39 GMT, Andy Goryachev wrote: > > the idea about mapping scroll events to the top/bottom of a cell is hinted > > in the code though > > could you please point to these places? I'm not sure I understand the question, as this is exactly what the PR is doing. The scrollToTop/scrollToBottom is replaced with a scroll of X pixels. - PR Comment: https://git.openjdk.org/jfx/pull/1326#issuecomment-2110436251
Re: RFR: 8329011: Update SQLite to 3.45.3
On Tue, 14 May 2024 14:23:18 GMT, Johan Vos wrote: > In Joeri I trust :) Me, too. :) - PR Comment: https://git.openjdk.org/jfx/pull/1454#issuecomment-2110417688
Re: RFR: 8329011: Update SQLite to 3.45.3
On Tue, 14 May 2024 13:23:06 GMT, Kevin Rushforth wrote: > @johanvos do you also want to review or is Joeri's review OK? In Joeri I trust :) - PR Comment: https://git.openjdk.org/jfx/pull/1454#issuecomment-2110380422
prism.maxvram limitation
Hi all, There is a long-lived bug in JavaFX: JDK-8089112: Need to handle the case of a failed texture load when rendering large images JavaFX manages a pool of resources that is limited to 512Mbytes by default. This limit can be increased by setting the 'prism.maxvram' system property. This limit is reasonable for the GraphicsPipelines (the D3DPipeline and the ES2Pipeline) that are backed by a graphics device that has such a limitation. But it does not make sense for pipelines that use only the main memory: the J2DPipeline and perhaps the SWPipeline. The J2DPipeline is currently used for printing. For example, printing an image on Letter paper using 600PPI printer requires 134_640_000 bytes. When the printed image is redirected to a PDF printer, even higher resolution is needed to get adequate quality because the PDF can zoom in. This often exceeds the limitation and ends in a NullPointerException if the allocated textures exceed the specified maxvram value. There is no way to specify different values for each pipeline or to remove the limitation for software pipelines. I would add this possibility? What do you think? Thanks, Eduard
Re: RFR: 8323511: Scrollbar Click jumps inconsistent amount of pixels [v6]
On Thu, 9 May 2024 14:49:26 GMT, Florian Kirmaier wrote: >> As seen in the unit test of the PR, when we click on the area above/below >> the scrollbar the position jumps - but the jump is now not always consistent. >> In the current version on the last cell - the UI always jumps to the top. In >> the other cases, the assumed default cell height is used. >> >> With this PR, always the default cell height is used, to determine how much >> is scrolled. >> This makes the behavior more consistent. >> >> Especially from the unit-test, it's clear that with this PR the behavior is >> much more consistent. >> >> This is also related to the following PR: >> https://github.com/openjdk/jfx/pull/1194 > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > JDK-8323511: Adjust javadoc of the getViewportLength() Ok, I've updated the CSR accrodingly together with Eduard. - PR Comment: https://git.openjdk.org/jfx/pull/1326#issuecomment-2110269379
Re: RFR: 8329011: Update SQLite to 3.45.3
On Tue, 14 May 2024 11:45:06 GMT, Hima Bindu Meda wrote: > Update SQLite to v3.45.3.Verified build on all platforms. Sanity testing > looks fine. Reviewers: @tiainen @kevinrushforth @johanvos do you also want to review or is Joeri's review OK? - PR Comment: https://git.openjdk.org/jfx/pull/1454#issuecomment-2110235435 PR Comment: https://git.openjdk.org/jfx/pull/1454#issuecomment-2110237796
Re: RFR: 8331748: Update libxml2 to 2.12.6
On Tue, 14 May 2024 05:21:37 GMT, Hima Bindu Meda wrote: > Updated libxml to v2.12.6. Verified build and sanity testing on all > platforms. No issue seen Reviewers: @tiainen @kevinrushforth - PR Comment: https://git.openjdk.org/jfx/pull/1453#issuecomment-2110233307
RFR: 8329011: Update SQLite to 3.45.3
Update SQLite to v3.45.3.Verified build on all platforms. Sanity testing looks fine. - Commit messages: - whitespace correction - Update sqlite to v3.45.3 Changes: https://git.openjdk.org/jfx/pull/1454/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1454&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8329011 Stats: 26451 lines in 4 files changed: 17765 ins; 2993 del; 5693 mod Patch: https://git.openjdk.org/jfx/pull/1454.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1454/head:pull/1454 PR: https://git.openjdk.org/jfx/pull/1454
Re: RFR: 8305418: [Linux] Replace obsolete XIM as Input Method Editor [v20]
On Mon, 13 May 2024 14:51:03 GMT, Thiago Milczarek Sayao wrote: > I will no longer commit to this change. Why? I'm getting ready to test this PR in the [HMCL](https://github.com/HMCL-dev/HMCL) community, and I expect it will address a lot of complaints from Linux users. HMCL is a popular app with hundreds of thousands of users, most of whom are from China. Although there are fewer Linux users among them, I believe this can get enough feedback. - PR Comment: https://git.openjdk.org/jfx/pull/1080#issuecomment-2109903935
