Re: RFR: 8272602: [macos] not all KEY_PRESSED events sent when control modifier is used
On Wed, 18 Aug 2021 23:57:38 GMT, Phil Race wrote: > When Ctrl+Space is pressed mac generates a string that contains the single > unicode code point zero. > The fn that converts it from an NSString to a Java String is using > NewStringUTF. > The input to that is a null terminated string which also has zero as the code > point it contains, so > we actually end up with a zero length Java string instead of the intended one > code point in length. > So the fix is to change the way we convert the string. > > There's an existing test CtrlAscii.java which sort of tests some of this but > it isn't asserting that you > get what you expect, its mostly testing you don't get something *unexpected* > .. it will happily pass if > you don't get keyevents. I did not want to change the purpose of that test > for this. > So I wrote a test specific to this Ctrl+Space to verify the fix but also ran > all the standard automated tests too. > On the other hand i now see that AFTER i did a multibyte character input > every time i switch into application with - i get following output in the > terminal: > 2021-08-19 08:44:24.155 java[18144:1448976] Apple AWT Internal Exception: > Java Exception It is not related to this change, i rolled it back and the issue persists so has nothing to do with this fix. - PR: https://git.openjdk.java.net/jdk/pull/5177
Re: RFR: 8272602: [macos] not all KEY_PRESSED events sent when control modifier is used
On Wed, 18 Aug 2021 23:57:38 GMT, Phil Race wrote: > When Ctrl+Space is pressed mac generates a string that contains the single > unicode code point zero. > The fn that converts it from an NSString to a Java String is using > NewStringUTF. > The input to that is a null terminated string which also has zero as the code > point it contains, so > we actually end up with a zero length Java string instead of the intended one > code point in length. > So the fix is to change the way we convert the string. > > There's an existing test CtrlAscii.java which sort of tests some of this but > it isn't asserting that you > get what you expect, its mostly testing you don't get something *unexpected* > .. it will happily pass if > you don't get keyevents. I did not want to change the purpose of that test > for this. > So I wrote a test specific to this Ctrl+Space to verify the fix but also ran > all the standard automated tests too. On the other hand i now see that AFTER i did a multibyte character input every time i switch into application with - i get following output in the terminal: 2021-08-19 08:44:24.155 java[18144:1448976] Apple AWT Internal Exception: Java Exception - PR: https://git.openjdk.java.net/jdk/pull/5177
Re: RFR: 8272602: [macos] not all KEY_PRESSED events sent when control modifier is used
On Wed, 18 Aug 2021 23:57:38 GMT, Phil Race wrote: > When Ctrl+Space is pressed mac generates a string that contains the single > unicode code point zero. > The fn that converts it from an NSString to a Java String is using > NewStringUTF. > The input to that is a null terminated string which also has zero as the code > point it contains, so > we actually end up with a zero length Java string instead of the intended one > code point in length. > So the fix is to change the way we convert the string. > > There's an existing test CtrlAscii.java which sort of tests some of this but > it isn't asserting that you > get what you expect, its mostly testing you don't get something *unexpected* > .. it will happily pass if > you don't get keyevents. I did not want to change the purpose of that test > for this. > So I wrote a test specific to this Ctrl+Space to verify the fix but also ran > all the standard automated tests too. Checked - fix works and it does not break input handling including multibyte characters. - Marked as reviewed by kizune (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5177
Integrated: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
On Wed, 11 Aug 2021 11:22:50 GMT, Alexander Zuev wrote: > Initial implementation and a test case. > > The problem is that Aqua LaF shows the focused component with the glow on the > border, hence when the border is not painted the foxus is not displayed. The > idea is to paint the glowing border on the focused component anyways. This pull request has now been integrated. Changeset: 481c1f05 Author: Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/481c1f0549e361e8e83fcf0530acc4ebdb60104b Stats: 121 lines in 2 files changed: 120 ins; 0 del; 1 mod 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked Reviewed-by: serb, psadhukhan - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
On Wed, 18 Aug 2021 04:59:36 GMT, Prasanta Sadhukhan wrote: > Have you checked if button does not have any text? Yes, i did. Neither my code nor native focus painter is affected by it. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v6]
> Initial implementation and a test case. > > The problem is that Aqua LaF shows the focused component with the glow on the > border, hence when the border is not painted the foxus is not displayed. The > idea is to paint the glowing border on the focused component anyways. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Restore Graphics color before leaving paintFocus() - Changes: - all: https://git.openjdk.java.net/jdk/pull/5082/files - new: https://git.openjdk.java.net/jdk/pull/5082/files/87ab3e3a..3b81c977 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5082=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5082=04-05 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5082/head:pull/5082 PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
On Mon, 16 Aug 2021 23:46:18 GMT, Sergey Bylokhov wrote: > You can trace the usage of graphics.getColor() for example in > AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color > property is usually saved and then restored. A few lines above paintFocus() is called there is a section that sets color to something different in case of opaque button and that does not affect the next step - text painting - since text painter will grab text color and set the graphics draw color to it. However in the interest of moving forward i will ad saving the color - after al it is not a performance critical task. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
On Mon, 16 Aug 2021 20:53:56 GMT, Sergey Bylokhov wrote: > What about the color property? That would be unnecessary, whoever does painting does not just assumes that the color is set to his preferred choice, everywhere in the code color is set up before doing any painting and i do not see anywhere that old color being preserved. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v5]
> Initial implementation and a test case. > > The problem is that Aqua LaF shows the focused component with the glow on the > border, hence when the border is not painted the foxus is not displayed. The > idea is to paint the glowing border on the focused component anyways. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Restore Graphics2D rendering state so we do not interfere with next stages of button painting. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5082/files - new: https://git.openjdk.java.net/jdk/pull/5082/files/df34ab0b..87ab3e3a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5082=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5082=03-04 Stats: 15 lines in 1 file changed: 13 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5082/head:pull/5082 PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
On Mon, 16 Aug 2021 18:49:31 GMT, Sergey Bylokhov wrote: > Don't you need to restore the properties of Graphics2D? I might as well. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
On Mon, 16 Aug 2021 02:15:55 GMT, Sergey Bylokhov wrote: > Can you please create of blend of two images, one for common focused > button(with border) and another one w/o. Just to check that the border is > drawn in the similar location and using similar "shape". Here is the overlap of two images - with and without borders painted, the without borders painted has 80% opacity so borders look faint. I do not think that the focus ring will always be pixel perfect match with the natively painted - but they are close enough that switching border painting on and off i can not tell difference visually and the new focus ring is always contained within the button shape so it will not disrupt anything beyond it. ![focus1b](https://user-images.githubusercontent.com/69642324/129612749-21283e4f-b97e-4aa0-aee1-451b9e60f151.png) - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
On Thu, 12 Aug 2021 18:51:10 GMT, Sergey Bylokhov wrote: > Probably solution should be somewhere similar to this one That works for icons but for text it would create a terrible mess, text with glow effect will be unreadable. I made code that draws the focus ring instead without drawing the rest of the border. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v3]
On Fri, 13 Aug 2021 12:15:38 GMT, Prasanta Sadhukhan wrote: > UIManager.getColor(). should suffice Fixed. > But I am not sure with this hardcoded values..Can't we leverage viewRect or > textRect to get the required coordinates? viewRect is set to be exactly (0, 0, b.width, b.height). Insets are buried deep inside JRS classes and used as hardcoded valued, i can not get them from there and both textRect and iconRect are not representing what needs to be drawn to be consistent with the way OS draws focus. Plus they are being initialized only later down the code and at the time of this call they are empty. Not that it would matter. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
> Initial implementation and a test case. > > The problem is that Aqua LaF shows the focused component with the glow on the > border, hence when the border is not painted the foxus is not displayed. The > idea is to paint the glowing border on the focused component anyways. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Remove getLookAndFeelDefaults() call - Changes: - all: https://git.openjdk.java.net/jdk/pull/5082/files - new: https://git.openjdk.java.net/jdk/pull/5082/files/fefcd37d..df34ab0b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5082=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5082=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5082/head:pull/5082 PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v3]
> Initial implementation and a test case. > > The problem is that Aqua LaF shows the focused component with the glow on the > border, hence when the border is not painted the foxus is not displayed. The > idea is to paint the glowing border on the focused component anyways. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Update copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/5082/files - new: https://git.openjdk.java.net/jdk/pull/5082/files/bda91782..fefcd37d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5082=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5082=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5082/head:pull/5082 PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v2]
> Initial implementation and a test case. > > The problem is that Aqua LaF shows the focused component with the glow on the > border, hence when the border is not painted the foxus is not displayed. The > idea is to paint the glowing border on the focused component anyways. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Changed to only paing focus ring without the entire border painting - Changes: - all: https://git.openjdk.java.net/jdk/pull/5082/files - new: https://git.openjdk.java.net/jdk/pull/5082/files/d4b21edc..bda91782 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5082=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5082=00-01 Stats: 9 lines in 1 file changed: 6 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5082/head:pull/5082 PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
On Thu, 12 Aug 2021 18:17:47 GMT, Sergey Bylokhov wrote: > There are two choices: draw this rectangle ourselves or request this feature > from the JRS. There i pushed the code that only draws the focus ring without the rest of the border. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
On Thu, 12 Aug 2021 02:15:06 GMT, Sergey Bylokhov wrote: > Painting the border while the user tries to disable the border via > setBorderPainted(false) does not look like a correct solution. The user might > be drawn something there already. I remember we discussed a similar a11y bug > related to focus and the only possible solution was to draw the focus > ourselves or request this feature in the JRS. There is no other way on Mac OS to indicate focused button but the glowing border. The same situation happens on Motif L - the only way to differentiate focused button from unfocused is by the border and in Motif - just as in Aqua - once border painting is disabled there is no way to tell if button is focused or not. User might draw something but if button becomes focused the focus indication will be drawn over it. If focus indicator disrupts user's painting on top of the button insets area user can always disable focus painting on such a component deliberately by calling JButton.setFocusPainted(boolean) - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
On Wed, 11 Aug 2021 15:16:22 GMT, Prasanta Sadhukhan wrote: > Does having 0,0 not create a problem in multiscreen environment? These coordinates are relative within the Graphics clip of the button so no, that does not cause any issue. Plus, if it would then button painting would be broken already since that's exactly what is passed to the border painter in case when borders are not switched off. - PR: https://git.openjdk.java.net/jdk/pull/5082
RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
Initial implementation and a test case. - Commit messages: - Test fixes - 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked Changes: https://git.openjdk.java.net/jdk/pull/5082/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5082=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269951 Stats: 99 lines in 2 files changed: 99 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5082/head:pull/5082 PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8267385: Create NSAccessibilityElement implementation for JavaComponentAccessibility [v9]
On Mon, 2 Aug 2021 09:00:14 GMT, Artem Semenov wrote: >> 8267385: Create NSAccessibilityElement implementation for >> JavaComponentAccessibility >> This pull request contains solutions for the following tickets: >> * JDK-8267385 Create NSAccessibilityElement implementation for >> JavaComponentAccessibility; >> * JDK-8262031 Create implementation for NSAccessibilityNavigableStaticText >> protocol; >> * JDK-8264287 Create implementation for NSAccessibilityComboBox protocol >> peer; >> * JDK-8264303 Create implementation for NSAccessibilityTabGroup protocol >> peer; >> * JDK-8264292 Create implementation for NSAccessibilityList protocol peer; >> * JDK-8267387 Create implementation for NSAccessibilityOutline protocol; >> * JDK-8267388 Create implementation for NSAccessibilityTable protocol. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > 1. Fixed problems with vertical navigation in table cells; > 2. I suggest doing the opposite, leave the CHECK_EXCEPTION and remove the > chech and the logging. The CHECK_EXCEPTION is better since it can be > configured via properties, and it will chech itself on what thread the > current code is executed. There are still some issues - namely, accessibility cursor got stuck inside combobox popup after popup is closed and voiceover shortcuts such as control option space announced in combobox but do nothing - but they can be fixed in separate bugs. Overall the new API is working and the functional difference can be overcome later in separate changes. - Marked as reviewed by kizune (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4412
[jdk17] Integrated: 8269984: [macos] JTabbedPane title looks like disabled
On Thu, 22 Jul 2021 16:25:40 GMT, Alexander Zuev wrote: > Fixed the version string parsing to work correctly on macOS 11.x and beyond This pull request has now been integrated. Changeset: 8adf008b Author: Alexander Zuev URL: https://git.openjdk.java.net/jdk17/commit/8adf008b150bfda783ae615965198edb4d196016 Stats: 27 lines in 1 file changed: 17 ins; 3 del; 7 mod 8269984: [macos] JTabbedPane title looks like disabled Reviewed-by: kcr, psadhukhan - PR: https://git.openjdk.java.net/jdk17/pull/269
Re: [jdk17] RFR: 8269984: [macos] JTabbedPane title looks like disabled [v2]
On Fri, 23 Jul 2021 04:16:10 GMT, Prasanta Sadhukhan wrote: > This is not required..Need to be removed. Done. - PR: https://git.openjdk.java.net/jdk17/pull/269
Re: [jdk17] RFR: 8269984: [macos] JTabbedPane title looks like disabled [v2]
> Fixed the version string parsing to work correctly on macOS 11.x and beyond Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Demoved isBigSurOrAbove variable that was never used in the code - Changes: - all: https://git.openjdk.java.net/jdk17/pull/269/files - new: https://git.openjdk.java.net/jdk17/pull/269/files/538305f9..3543c209 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=269=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=269=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk17/pull/269.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/269/head:pull/269 PR: https://git.openjdk.java.net/jdk17/pull/269
Re: [jdk17] RFR: 8269984: [macos] JTabbedPane title looks like disabled
On Fri, 23 Jul 2021 04:18:42 GMT, Prasanta Sadhukhan wrote: > Since existing currentMacOSXVersionMatchesGivenVersionRange() is changed, did > you test in osx 10.15 or below to see current functionality is ok? I have tested the public methods of this class with simulated version strings for previous macOS releases - they return correct assumptions about them. I haven't tested the code that acquires the version string but i haven't changed that. > ALso, please add noreg-demo to JBS since I guess SwingSet2 is used to test. Done. - PR: https://git.openjdk.java.net/jdk17/pull/269
Re: [jdk17] RFR: 8015886: java/awt/Focus/DeiconifiedFrameLoosesFocus/DeiconifiedFrameLoosesFocus.java sometimes failed on ubuntu [v2]
On Thu, 22 Jul 2021 20:49:36 GMT, Alexander Zvegintsev wrote: >> This test can fail pretty consistently on some slow systems. Increased delay >> fixes the issue. >> We do have similar fix in #260 > > Alexander Zvegintsev has updated the pull request incrementally with one > additional commit since the last revision: > > year bump Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/272
Re: [jdk17] RFR: 8015886: java/awt/Focus/DeiconifiedFrameLoosesFocus/DeiconifiedFrameLoosesFocus.java sometimes failed on ubuntu
On Thu, 22 Jul 2021 19:53:37 GMT, Alexander Zvegintsev wrote: > This test can fail pretty consistently on some slow systems. Increased delay > fixes the issue. > We do have similar fix in #260 I am not sure if we supposed to bump up the copyright year or it is supposed to be done with some script later, but i usually do it myself just to be sure. - PR: https://git.openjdk.java.net/jdk17/pull/272
[jdk17] RFR: 8269984: [macos] JTabbedPane title looks like disabled
Fixed the version string parsing to work correctly on macOS 11.x and beyond - Commit messages: - 8269984: [macos] JTabbedPane title looks like disabled Changes: https://git.openjdk.java.net/jdk17/pull/269/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=269=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269984 Stats: 26 lines in 1 file changed: 17 ins; 2 del; 7 mod Patch: https://git.openjdk.java.net/jdk17/pull/269.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/269/head:pull/269 PR: https://git.openjdk.java.net/jdk17/pull/269
Withdrawn: 8269984: [macos] JTabbedPane title looks like disabled
On Tue, 20 Jul 2021 01:11:25 GMT, Alexander Zuev wrote: > Fixed the version string parsing to work correctly on macOS 11.x and beyond This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/4833
Re: RFR: 8269984: [macos] JTabbedPane title looks like disabled
On Tue, 20 Jul 2021 01:11:25 GMT, Alexander Zuev wrote: > Fixed the version string parsing to work correctly on macOS 11.x and beyond Closing this PR while opening one for jdk17. - PR: https://git.openjdk.java.net/jdk/pull/4833
Re: RFR: 8267385: Create NSAccessibilityElement implementation for JavaComponentAccessibility [v5]
On Thu, 15 Jul 2021 12:07:44 GMT, Artem Semenov wrote: >> 8267385: Create NSAccessibilityElement implementation for >> JavaComponentAccessibility >> This pull request contains solutions for the following tickets: >> * JDK-8267385 Create NSAccessibilityElement implementation for >> JavaComponentAccessibility; >> * JDK-8262031 Create implementation for NSAccessibilityNavigableStaticText >> protocol; >> * JDK-8264287 Create implementation for NSAccessibilityComboBox protocol >> peer; >> * JDK-8264303 Create implementation for NSAccessibilityTabGroup protocol >> peer; >> * JDK-8264292 Create implementation for NSAccessibilityList protocol peer; >> * JDK-8267387 Create implementation for NSAccessibilityOutline protocol; >> * JDK-8267388 Create implementation for NSAccessibilityTable protocol. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > ok, so this is in the new code, how hard it will be to reimplement these > methods to call invokeAndWait only once per callback from the native, as it > usually was done before? Ok, remaining issues for the table implementation: 1. Debug information "WARNING: -[CommonComponentAccessibility isAccessibilityEnabled] called on component that has no accessible component..." printed whenever navigated to the cell that has image inside it - i think this as well as the same message when navigating with voiceover shortcuts to the disabled components - needs to be addressed in this pull request 2. No matter what selection mode is set on the table - cell selection, row selection or column selection - when selection changes due to the table navigation the content of the row where cursor currently placed is announced. Since in previous implementation we had only "Selection changes: n elements out of nnn elements are selected" announcement and in individual cell selection mode there were no announcements at all i consider it as not a functional regression but still we need to create a bug for improving this behavior and it should be addressed as soon as possible and backported to all releases where this implementation will go. 3. Indexes when announcing the content are not right - when full cell announcement happens it always says column 1 row 1 - no matter what cell is being announced. I think that needs to be fixed. - PR: https://git.openjdk.java.net/jdk/pull/4412
Integrated: 8269637: javax/swing/JFileChooser/FileSystemView/SystemIconTest.java fails on windows
On Wed, 14 Jul 2021 11:19:53 GMT, Alexander Zuev wrote: > Make fallback code for inaccessible file to return multiresolution icon > Remove test from ProblemList This pull request has now been integrated. Changeset: a033866d Author:Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/a033866d786507db69ab75643684e617fd1f4ba2 Stats: 11 lines in 2 files changed: 8 ins; 1 del; 2 mod 8269637: javax/swing/JFileChooser/FileSystemView/SystemIconTest.java fails on windows Reviewed-by: aivanov - PR: https://git.openjdk.java.net/jdk/pull/4777
Re: RFR: 8269637: javax/swing/JFileChooser/FileSystemView/SystemIconTest.java fails on windows
On Wed, 14 Jul 2021 11:56:15 GMT, Alexey Ivanov wrote: > So, essentially all the icons returned are MultiResolutionIcon even if > there's only one icon, right? It was a multiresolution icon even with one icon inside but only when the native and requested icon resolutions do not match. Now it will always be MRI even if file is inaccessible and there is a default icon in System32.dll that matches the requested resolution. - PR: https://git.openjdk.java.net/jdk/pull/4777
Re: RFR: 8267385: Create NSAccessibilityElement implementation for JavaComponentAccessibility [v2]
On Fri, 2 Jul 2021 12:45:25 GMT, Artem Semenov wrote: >> 8267385: Create NSAccessibilityElement implementation for >> JavaComponentAccessibility >> This pull request contains solutions for the following tickets: >> * JDK-8267385 Create NSAccessibilityElement implementation for >> JavaComponentAccessibility; >> * JDK-8262031 Create implementation for NSAccessibilityNavigableStaticText >> protocol; >> * JDK-8264287 Create implementation for NSAccessibilityComboBox protocol >> peer; >> * JDK-8264303 Create implementation for NSAccessibilityTabGroup protocol >> peer; >> * JDK-8264292 Create implementation for NSAccessibilityList protocol peer; >> * JDK-8267387 Create implementation for NSAccessibilityOutline protocol; >> * JDK-8267388 Create implementation for NSAccessibilityTable protocol. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > Even on Linux? I meant if the test will be run on the platform where a11y > is not implemented then this will be During testing of text input i have found that the previous implementation with voiceover active pronounced every entered character and then, whet typing pauses, it pronounced the last entered word. With new implementation i only hear the first typed character in the text field and then nothing - no other characters and no last entered word. Looks like we are not sending some notification to the system when text is updated. Needs further investigation. - PR: https://git.openjdk.java.net/jdk/pull/4412
Re: RFR: 8267385: Create NSAccessibilityElement implementation for JavaComponentAccessibility
On Tue, 8 Jun 2021 13:06:53 GMT, Artem Semenov wrote: > 8267385: Create NSAccessibilityElement implementation for > JavaComponentAccessibility > This pull request contains solutions for the following tickets: > * JDK-8267385 Create NSAccessibilityElement implementation for > JavaComponentAccessibility; > * JDK-8262031 Create implementation for NSAccessibilityNavigableStaticText > protocol; > * JDK-8264287 Create implementation for NSAccessibilityComboBox protocol peer; > * JDK-8264303 Create implementation for NSAccessibilityTabGroup protocol peer; > * JDK-8264292 Create implementation for NSAccessibilityList protocol peer; > * JDK-8267387 Create implementation for NSAccessibilityOutline protocol; > * JDK-8267388 Create implementation for NSAccessibilityTable protocol. src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 372: > 370: if ([javaRole isEqualToString:@"pagetablist"]) { > 371: newChild = [TabGroupLegacyAccessibility alloc]; > 372: } else if ([javaRole isEqualToString:@"table"]) { Do we even need legacy peer implementation in the new classes? - PR: https://git.openjdk.java.net/jdk/pull/4412
Re: RFR: 8267385: Create NSAccessibilityElement implementation for JavaComponentAccessibility
On Tue, 8 Jun 2021 13:06:53 GMT, Artem Semenov wrote: > 8267385: Create NSAccessibilityElement implementation for > JavaComponentAccessibility > This pull request contains solutions for the following tickets: > * JDK-8267385 Create NSAccessibilityElement implementation for > JavaComponentAccessibility; > * JDK-8262031 Create implementation for NSAccessibilityNavigableStaticText > protocol; > * JDK-8264287 Create implementation for NSAccessibilityComboBox protocol peer; > * JDK-8264303 Create implementation for NSAccessibilityTabGroup protocol peer; > * JDK-8264292 Create implementation for NSAccessibilityList protocol peer; > * JDK-8267387 Create implementation for NSAccessibilityOutline protocol; > * JDK-8267388 Create implementation for NSAccessibilityTable protocol. src/java.desktop/macosx/classes/sun/lwawt/macosx/CAccessibility.java line 728: > 726: // Each child takes up three entries in the array: one for itself, > one for its role, and one for the recursion level > 727: public static Object[] getChildrenAndRolesRecursive(final Accessible > a, final Component c, final int whichChildren, final boolean allowIgnored, > final int level) { > 728: if (a == null) return null; I am not sure but i think CAccessible is considered a public API so adding public methods to it might require a CSR filling. - PR: https://git.openjdk.java.net/jdk/pull/4412
Re: RFR: 8264305: Create implementation for native accessibility peer for Statusbar java role
On Wed, 2 Jun 2021 13:58:24 GMT, Pankaj Bansal wrote: > Create implementation for native accessibility peer for Statusbar java role. > I do not see any Swing component with StatusBar accessibility role, so I have > created a component with the StatusBar role to test this. I have tested the > implementation with the following example and a few more similar examples. I > do not see any difference in Voice Over output. > ` > > import javax.accessibility.Accessible; > import javax.accessibility.AccessibleContext; > import javax.accessibility.AccessibleRole; > import javax.accessibility.AccessibleStateSet; > import javax.swing.JFrame; > import javax.swing.JLabel; > import javax.swing.JPanel; > import javax.swing.SwingUtilities; > import javax.swing.Timer; > import javax.swing.border.BevelBorder; > import java.awt.BorderLayout; > import java.awt.Color; > import java.awt.Dimension; > import java.awt.event.ActionEvent; > import java.awt.event.ActionListener; > import java.text.DateFormat; > > public class StatusBarDemo { > > public static void main(String[] args) throws Exception { > StatusBarDemo demo = new StatusBarDemo(); > SwingUtilities.invokeAndWait(demo::createAndShowGUI); > } > > void createAndShowGUI() { > JFrame jframe = new JFrame("StatusBar Demo"); > jframe.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); > jframe.setLayout(new BorderLayout()); > jframe.setSize(200, 200); > > // create the status bar panel and shove it down the bottom of > the frame > JPanel statusPanel = new StatusBar(); > statusPanel.setBorder(new BevelBorder(BevelBorder.LOWERED)); > statusPanel.setPreferredSize(new Dimension(jframe.getWidth(), > 16)); > statusPanel.setLayout(new BorderLayout()); > > JLabel date = new JLabel(); > date.setOpaque(true); > date.setBackground(Color.black); > date.setForeground(Color.WHITE); > statusPanel.setLayout(new BorderLayout()); > statusPanel.setBackground(Color.LIGHT_GRAY); > statusPanel.add(date, BorderLayout.CENTER); > > > Timer timer = new javax.swing.Timer(1000, new ActionListener() { > @Override > public void actionPerformed(ActionEvent e) { > java.util.Date now = new java.util.Date(); > String ss = DateFormat.getDateTimeInstance().format(now); > date.setText(ss); > date.setToolTipText("Welcome, Today is " + ss); > > } > }); > > timer.start(); > > jframe.add(statusPanel, BorderLayout.SOUTH); > > jframe.setLocationRelativeTo(null); > jframe.setVisible(true); > } > > class StatusBar extends JPanel implements Accessible { > > AccessibleContext accessibleContext; > > public AccessibleContext getAccessibleContext() { > if (accessibleContext == null) { > accessibleContext = new StatusBar.AccessibleJStatusBar(); > } > return accessibleContext; > } > > protected class AccessibleJStatusBar extends AccessibleJComponent > { > public AccessibleStateSet getAccessibleStateSet() { > AccessibleStateSet states = super.getAccessibleStateSet(); > return states; > } > public AccessibleRole getAccessibleRole() { > return AccessibleRole.STATUS_BAR; > } > } > } > } > > > ` Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4307
Integrated: 8182043: Access to Windows Large Icons
On Mon, 8 Mar 2021 13:22:07 GMT, Alexander Zuev wrote: > Fix updated after first round of review. This pull request has now been integrated. Changeset: 7f52c50b Author: Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/7f52c50ba32eecf5f379f8db30ac6a5cc50b3b66 Stats: 408 lines in 6 files changed: 328 ins; 39 del; 41 mod 8182043: Access to Windows Large Icons Reviewed-by: aivanov, azvegint, prr - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v15]
On Thu, 27 May 2021 16:16:19 GMT, Sergey Bylokhov wrote: > Is this requirement is so important? can we return an MRI(same as on Windows) > which will have just one resolution? We might - when the implementation will be done on other platforms. Probably it will be done by me, probably - by someone else. Right now we return whatever we have so on Linux it is UIManager default icons for file or a folder (which is exactly what any file manager on Linux shows for any file and this is exactly what we promised in the method description). In the future it can change but for now it is all we can guarantee. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v16]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Test comment fixed. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/9e7bf901..06521978 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=15 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=14-15 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v15]
On Wed, 26 May 2021 21:06:49 GMT, Sergey Bylokhov wrote: > Did somebody run the test on Linux and macOS, Does our stub implementation > there confirms the specification? I did run fill tier4 and visually compared the JFileChooser appearance on both Linux and Mac OS X - this change introduced no visual degradation to them and the test passed. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v15]
On Wed, 26 May 2021 16:52:38 GMT, Alexander Zuev wrote: >> Fix updated after first round of review. > > Alexander Zuev has updated the pull request incrementally with two additional > commits since the last revision: > > - Fixed small errors >Adjustments in the test > - Grammar fix in method documentation > Why does it need to do this ? I'm testing the implementation here and i know that on Windows for these well known files and folders it is possible to get icons of all the sizes mentioned in the test and we will return the correct icon size - so that's what i'm testing it this way. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v15]
On Thu, 27 May 2021 00:18:13 GMT, Phil Race wrote: >> Alexander Zuev has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Fixed small errors >>Adjustments in the test >> - Grammar fix in method documentation > > test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 28: > >> 26: * @bug 8182043 >> 27: * @summary Access to Windows Large Icons >> 28: * sun.awt.shell.ShellFolder > > What's the comment here mean ? That's a leftover from the first iteration of the test. I guess i have to remove it. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v15]
On Wed, 26 May 2021 21:12:16 GMT, Sergey Bylokhov wrote: >> Alexander Zuev has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Fixed small errors >>Adjustments in the test >> - Grammar fix in method documentation > > test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 77: > >> 75: int[] sizes = new int[] {16, 32, 48, 64, 128}; >> 76: for (int size : sizes) { >> 77: ImageIcon icon = (ImageIcon) fsv.getSystemIcon(file, size, >> size); > > Is this cast allowed without instanceof check? On Windows - yes. When implementation is complete on other platforms test will have to be updated depending on the way it will be implemented there. > test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 87: > >> 85: } >> 86: >> 87: if (implComplete && icon.getIconWidth() != size) { > > Why the size of the returned images might be different? The spec state the > size parameter in the getSystemIcon is the size of the icon in the userspace. The spec says that we are taking into consideration requested sizes but depending on the platform implementation exact match can not be guaranteed. On Windows it can so i'm checking for that. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v15]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with two additional commits since the last revision: - Fixed small errors Adjustments in the test - Grammar fix in method documentation - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/b3ca9da6..9e7bf901 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=14 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=13-14 Stats: 17 lines in 2 files changed: 3 ins; 6 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v14]
On Wed, 26 May 2021 16:03:02 GMT, Alexander Zuev wrote: >> test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 67: >> >>> 65: if (!gotException) { >>> 66: throw new RuntimeException("Negative size icon should throw >>> invalid argument exception"); >>> 67: } >> >> A suggestion: throw the exception inside try-block and ignore the expected >> exception, then `gotException` can be dropped. >> Suggestion: >> >> try { >> fsv.getSystemIcon(new File("."), -1, 16); >> throw new RuntimeException("Negative size icon should throw >> invalid argument exception"); >> } catch (IllegalArgumentException iae) { >> // expected >> } >> >> >> Shall the test also exercise 0 as an invalid parameter? Shall the test also >> pass an invalid height? > > Fixed. I do not think such detailed testing is required. After all it is not a full spec conformance test. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v14]
On Wed, 26 May 2021 15:07:30 GMT, Alexey Ivanov wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> null file now properly causes IllegalArgumentException >> Small fixed in JavaDoc > > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 296: > >> 294: >> 295: if (f == null){ >> 296: throw new IllegalArgumentException("File reference should >> be specified"); > > Shall the exception message be more specific: "The file reference should not > be null"? Ficed. > test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 58: > >> 56: >> 57: static void negativeTests() { >> 58: ImageIcon icon; > > Can it be icon? This test doesn't use the fact that the returned object is > `ImageIcon`. Fixed. > test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 67: > >> 65: if (!gotException) { >> 66: throw new RuntimeException("Negative size icon should throw >> invalid argument exception"); >> 67: } > > A suggestion: throw the exception inside try-block and ignore the expected > exception, then `gotException` can be dropped. > Suggestion: > > try { > fsv.getSystemIcon(new File("."), -1, 16); > throw new RuntimeException("Negative size icon should throw > invalid argument exception"); > } catch (IllegalArgumentException iae) { > // expected > } > > > Shall the test also exercise 0 as an invalid parameter? Shall the test also > pass an invalid height? Fixed. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8264302: Create implementation for Accessibility native peer for Splitpane java role [v2]
On Wed, 26 May 2021 04:53:41 GMT, Pankaj Bansal wrote: >> Create implementation for Accessibility native peer for Splitpane java role. >> I have tested it using the SwingSet2 and splitpane demo from >> https://docs.oracle.com/javase/tutorial/uiswing/examples/components/index.html#SplitPaneDemo. >> The do not see any inconsistency in Vo behaviour before and after the >> changes. > > Pankaj Bansal has updated the pull request incrementally with one additional > commit since the last revision: > > Correct small issue Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4181
Re: RFR: 8264302: Create implementation for Accessibility native peer for Splitpane java role
On Tue, 25 May 2021 08:24:21 GMT, Pankaj Bansal wrote: > Create implementation for Accessibility native peer for Splitpane java role. > I have tested it using the SwingSet2 and splitpane demo from > https://docs.oracle.com/javase/tutorial/uiswing/examples/components/index.html#SplitPaneDemo. > The do not see any inconsistency in Vo behaviour before and after the changes. src/java.desktop/macosx/native/libawt_lwawt/awt/a11y/SplitpaneAccessibility.m line 29: > 27: > 28: /* > 29: * Implementation of the accessibility peer for the ScrollBar role ScrollBar? I think you meant something else. - PR: https://git.openjdk.java.net/jdk/pull/4181
Re: RFR: 8182043: Access to Windows Large Icons [v13]
On Tue, 25 May 2021 23:17:39 GMT, Phil Race wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Changed API to support non-square icons and updated documentation >> accordingly >> Fixed test to support updated API >> Added negative test cases > > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 282: > >> 280: * @return an icon as it would be displayed by a native file chooser >> 281: * or null if invalid parameters are passed such as reference to a >> 282: * non-existing file. > > non-existent not non-existing, but I think null deserves an IAE - as I had > written a couple of days ago. > > I see you dropped the words about null if the size is too large. > Should I interpret that as meaning one of the things I suggested - that this > is just another case of a closest match ? Thanks for correction. Yes - we will return the best match no matter how big the requested size is. Fixed IAE in case of null file. > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 286: > >> 284: * @see AbstractMultiResolutionImage >> 285: * @see FileSystemView#getSystemIcon(File) >> 286: * @since 17 > > You need to add the IAE to the javadoc. Done. > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 290: > >> 288: public Icon getSystemIcon(File f, int width, int height) { >> 289: if (height <1 || width < 1) { >> 290: throw new IllegalArgumentException("Icon size can not be >> below 1"); > > (minor) spacing. <1 Typo. Fixed. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v14]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: null file now properly causes IllegalArgumentException Small fixed in JavaDoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/d679dc01..b3ca9da6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=12-13 Stats: 9 lines in 1 file changed: 6 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v12]
On Fri, 21 May 2021 22:07:30 GMT, Phil Race wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Chenged implementation specification based on CSR review > > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 280: > >> 278: * @param size width and height of the icon in user coordinate >> system. >> 279: * This API only supports square icons with the edge length >> 280: * equals or greater than 1. Maximum size allowed is defined by the > > I think this is a weird way to say size >=1. > > We should throw IAE for <=0 . > > I also think we need to allow for rectangular icons, not bake it in to the > spec in such strong terms that we don't. > I suppose it could be revised if a platform that needs it comes along but > ... > > Still not sure about failing if you exceed the maximum size. > > Why ? Why not just return the largest possible - it is the same thing as > other cases, we'll return the best match. > > If that is what you already mean, then say it more clearly. Ok, in order to allow rectangular items API needs to be slightly modified and tests and documentation should be amended accordingly. I did that, please take a look, once it is settled i will update and finalize CSR. - PR: https://git.openjdk.java.net/jdk/pull/2875
Integrated: 8267066: New NSAccessibility peers should return they roles and subroles directly
On Mon, 24 May 2021 11:03:35 GMT, Alexander Zuev wrote: > Made all implementing classes return their roles directly. > Change nullability attributes to the singular style across new classes. This pull request has now been integrated. Changeset: d86f9164 Author:Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/d86f91642d7dcf829b8093ebed336d4386bff67a Stats: 78 lines in 25 files changed: 42 ins; 0 del; 36 mod 8267066: New NSAccessibility peers should return they roles and subroles directly Reviewed-by: pbansal - PR: https://git.openjdk.java.net/jdk/pull/4162
Re: RFR: 8267066: New NSAccessibility peers should return they roles and subroles directly [v2]
On Tue, 25 May 2021 05:38:19 GMT, Pankaj Bansal wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fixed old style nullability attributes in StaticTextAccessibility.h > > src/java.desktop/macosx/native/libawt_lwawt/awt/a11y/CheckboxAccessibility.h > line 32: > >> 30: }; >> 31: - (NSAccessibilityRole _Nonnull)accessibilityRole; >> 32: - (id _Nonnull)accessibilityValue; > > Looks like the value can be null. No? Not here. The value here is a state of the checkbox - checked or not checked, unlike text components where value can be set to null, here value is being generated from the component state and can not be null. > src/java.desktop/macosx/native/libawt_lwawt/awt/a11y/RadiobuttonAccessibility.h > line 32: > >> 30: }; >> 31: - (NSAccessibilityRole _Nonnull)accessibilityRole; >> 32: - (id _Nonnull)accessibilityValue; > > Same as earlier. Can't this be null? Same here, value represents the state of the control and can not be set to null. > src/java.desktop/macosx/native/libawt_lwawt/awt/a11y/StaticTextAccessibility.h > line 39: > >> 37: - (NSAccessibilityRole _Nonnull)accessibilityRole; >> 38: - (nullable NSString >> *)accessibilityAttributedStringForRange:(NSRange)range; >> 39: - (nullable NSString *)accessibilityValue; > > You missed these here in .h file Yes. Fixed. - PR: https://git.openjdk.java.net/jdk/pull/4162
Re: RFR: 8267066: New NSAccessibility peers should return they roles and subroles directly [v2]
> Made all implementing classes return their roles directly. > Change nullability attributes to the singular style across new classes. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Fixed old style nullability attributes in StaticTextAccessibility.h - Changes: - all: https://git.openjdk.java.net/jdk/pull/4162/files - new: https://git.openjdk.java.net/jdk/pull/4162/files/c6a0cbc0..3cd06156 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4162=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4162=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4162.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4162/head:pull/4162 PR: https://git.openjdk.java.net/jdk/pull/4162
Integrated: 8208747: [a11y] [macos] In Optionpane Demo, inside ComponentDialog Example, unable to navigate to all items, with VO on
On Tue, 18 May 2021 09:05:10 GMT, Alexander Zuev wrote: > Added accessibilityIndex function that correctly returns the index of a > child in parent container This pull request has now been integrated. Changeset: 49f622c9 Author:Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/49f622c9473464bf1d1f33b88abf46adadd9fa1c Stats: 8 lines in 1 file changed: 8 ins; 0 del; 0 mod 8208747: [a11y] [macos] In Optionpane Demo, inside ComponentDialog Example, unable to navigate to all items, with VO on Reviewed-by: pbansal - PR: https://git.openjdk.java.net/jdk/pull/4084
RFR: 8267066: New NSAccessibility peers should return they roles and subroles directly
Made all implementing classes return their roles directly. Change nullability attributes to the singular style across new classes. - Commit messages: - 8267066: New NSAccessibility peers should return they roles and subroles directly Changes: https://git.openjdk.java.net/jdk/pull/4162/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4162=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267066 Stats: 76 lines in 25 files changed: 42 ins; 0 del; 34 mod Patch: https://git.openjdk.java.net/jdk/pull/4162.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4162/head:pull/4162 PR: https://git.openjdk.java.net/jdk/pull/4162
Re: RFR: 8208747: [a11y] [macos] In Optionpane Demo, inside ComponentDialog Example, unable to navigate to all items, with VO on
On Thu, 20 May 2021 12:29:10 GMT, Pankaj Bansal wrote: > What I am asking is why the same issue is not observed in JList. In JList it is observed differently because there the multiple selection is allowed and because of that fever events about items selected/deselected and focus change being generated. When we move cursor with the VO hot keys we got selection change events with currently chosen item and the first item in the list, that leads to adding of the chosen item to the selection. That does not change focus and focus event is not being generated but instead of moving cursor we just adjusting selection. But if you will try to move cursor with simple up/down arrows you will see that you can not navigate past second or third item - eventually the order of selection/focus events will bring it back to the beginning of the list. It all starts with the discrepancy between the container reporting which children is selected and the index of the item in parent container reported by the selected children. - PR: https://git.openjdk.java.net/jdk/pull/4084
Re: RFR: 8264304: Create implementation for NSAccessibilityToolbar protocol peer [v2]
On Thu, 20 May 2021 10:44:10 GMT, Pankaj Bansal wrote: >> Create implementation for accessibility native peer for Toolbar component. >> There is no direct protocol for this role and there is no protocol in list >> of role based protocols defined by Apple Accessibility API, which can be >> used for this role directly. >> https://developer.apple.com/library/archive/documentation/Accessibility/Conceptual/AccessibilityMacOSX/ImplementingAccessibilityforCustomControls.html. >> So custom implementation of peer is created. >> >> Accessibility support in JToolBar is very trivial, so the native peer is >> also simple. >> >> I have tested this implementation by using the SwingSet2 demo and Toolbar >> samples present here >> https://docs.oracle.com/javase/tutorial/uiswing/components/toolbar.html. The >> Voice Over works fine and consistent with the old implementation. > > Pankaj Bansal 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' into JDK-8264304 > - 8264304: Create implementation for NSAccessibilityToolbar protocol peer Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4040
Re: RFR: 8182043: Access to Windows Large Icons [v11]
On Fri, 21 May 2021 21:24:10 GMT, Phil Race wrote: > if the aspect ratio is consistent as I expect is typical we should support > that Ok, i see your point. I can do that for sure, it is just a change of API for now, no implementation will be affected since Windows does not support non-square icons for files and passage about exact size can not be guaranteed will save us from "i requested 128x20 and you returned 128x128" complaints. One will get what platform is able to serve. However, i do not see why we should limit the maximum requested size. > I can't agree that IAE is wrong for a negative size. How about non-existing file or file? Also IAE? - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v12]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Chenged implementation specification based on CSR review - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/548dcef1..56285783 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=10-11 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v11]
On Fri, 21 May 2021 20:10:55 GMT, Phil Race wrote: > Please note that I added a number of comments in the CSR itself. > I had suggestions about what to do in many of the cases but there's a couple > of cases where > I could not either divine what was intended or know the best way to phrase it. I am responding to CSR comments right now but honestly i would prefer to do a review in one place - preferably here. For some reason i do not receive any notifications from JBS about new comments in CSR so i had to monitor it manually by refreshing the page. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v7]
On Fri, 21 May 2021 19:11:45 GMT, Alexey Ivanov wrote: >> But how you got them via this method? I am not sure what parameters should >> be passed to it. > > Didn't you answer your question already? If `FileSystemView.getRoots()` > returns Desktop in Windows shell namespace. Otherwise, I don't know a way to > get a reference to a *virtual* folder in Windows shell namespace which > doesn't reference a file system object. > > Alex's example uses "3D Objects" folder which is one of the known folders and > has its own icon instead of the standard folder icon. > > It's possible that I don't understand the question clearly. > > Alex's fix affects WindowsPlacesBar on the left of JFileChooser in Windows > LaF, the icons there look crispier in High DPI environment. > But how you got them via this method? I am not sure what parameters should be > passed to it. ` String userdir = System.getenv("userprofile"); Icon icon = FileSystemView.getFileSystemView().getSystemIcon(new File(userdir + "\\3D Objects"), 64); ` For some of the libraries getting file reference is quite easy, for some - not so much. But still doable. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v7]
On Fri, 21 May 2021 05:42:46 GMT, Sergey Bylokhov wrote: > It is accessible from the "JFileChooser" drop-down menu. On my system, the > Libraries at that drop down menu contain "GIT", "Documents", "Music". > https://docs.microsoft.com/en-us/windows/client-management/windows-libraries > > There are also "known folders" such as "Network" or "My computer" which do > not have a real path but can be navigated in the JFileChooser and has an icon. Ok, got it. Well, since they can be translated into the file system paths - even if these paths do not belong to physical filesystem - they are supported by the new API. Not for all of them i am able to receive the high resolution icons - like "Recent Items" for some reason only provides 32x32 and 16x16 no matter what size i am asking for. Others such as Documents, My Computer or 3D Objects do provide all resolutions available. For example here's the screenshot of all the available icons for 3D Objects ![image](https://user-images.githubusercontent.com/69642324/119092517-62a9f980-b9c3-11eb-9d51-b7745cab564c.png) - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v7]
On Fri, 21 May 2021 03:21:19 GMT, Sergey Bylokhov wrote: >> The JFileChooser supports the libraries since it allows navigation inside >> them. It is done via ShellFolder which extends the file class. The >> FileSystemView feeds the JFileChooser by the data, so it may returns >> "non-file" objects. >> For example the FileSystemView.getRoots() on WIndows will return "new >> Win32ShellFolder2(DESKTOP)". >> >> Usually, when the FileSystemView gets a file object it checks that the file >> is "instance ShellFolder". >> For example, in the newly added method the line "sf = >> ShellFolder.getShellFolder(f);" will check that the file is ShellFolder and >> return it as-is without checking of file existence, and really that path may >> not exist. but if it is a plain file it will check that. >> >> This is why I have asked about virtual folders, do we test them or we can >> consider them as "not existed"? > > Maybe we can test this case by just reusing this method where the > "sf.getIcon(largeicon)" previously used in the WindowsPlacesBar class? That > old method has the special code for "folder.isLibrary()" so I assume it > supported the Libraries. And if we will start to use the new method then all > old tests for the JFileChooser will start to test it. What is the library a collection folder such as Recent Items or Documents? - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 22:38:48 GMT, Sergey Bylokhov wrote: > But this is shared code, which has a specification it should work everywhere, > so even if on Linux and macOS it is not implemented in the best way it should > return something. The stuff that is returned by the common code is already tested in UIManager tests. This issue is windows specific. This implementation at this moment of time is windows specific. Once we extend implementation - and i mean real implementation with support for multiple resolution icons - this test will be updated to reflect it. This is not a specification conformance test, it is implementation logic test. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 22:37:08 GMT, Sergey Bylokhov wrote: > I did not get how you will be able to force use MRI later since this method > is implemented as a return icon. So this choice should be made before > integration. I am not going to force MRI usage - i am assuming that there might be an implementation that does not do it hence i am going to go to the lowest common denominator - Icon. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 19:35:57 GMT, Phil Race wrote: > Really I would need to see what the actual delta ends up being to be sure for > this case. I have updated the method documentation. Could you please take a look before i finalized the CSR again? I am really trying to push this functionality into 17 and there's not much time left. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v11]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Specification of getSystemIcon reworded to get rid of the non-public class reference and to better describe some cornor cases - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/09c7f8d7..548dcef1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=09-10 Stats: 11 lines in 1 file changed: 5 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 17:29:55 GMT, Sergey Bylokhov wrote: > Here I am not talking about specification, I am talking about the usage of > it, is the instanceof+cast is helpful? It is necessary until all the consequences of always returning of MRI is extensively tested on all the supported platforms and it does not cause any serious regression or visual degradations. So for now i would not enforce MRI usage in this method which means use base image class instead and do the cast later. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 17:33:08 GMT, Sergey Bylokhov wrote: > But it is still part of the specification unlike implnote/apinote I think you can suggest usage of the implNote here - i am going from the initial description of the reason implSpec in the JEP saying that implementation and logic of it may vary between different Java SE implementations and even between different platforms so i am going with the original reasoning for implSpec tag existence. If you disagree, please file the separate issue for spec amendment once this PR is integrated. Or we can discuss it and i file follow-up bug - whatever you prefer, but i honestly think it is not a blocker and that this technical issue linger in this state for way too long. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 16:09:14 GMT, Sergey Bylokhov wrote: > Other than using "explorer" what prevents us to run this test on other > platforms? Because right now it makes no sense? The functionality we are testing is implemented on Windows only. Once we extend it to other platforms we might change the test. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 16:14:42 GMT, Sergey Bylokhov wrote: > BTW this is why I recommended filing a CSR after the fix is fully discussed > and agreed upon. The CSR was filed almost five years ago. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 15:57:04 GMT, Sergey Bylokhov wrote: > This is to describe one of my questions above, is this instanceof+cast cannot > be improved? Why we cannot always wrap the data in the MRI and if we have > only one icon return the MRI with one resolution? No because in specification we say that we do not guarantee the the icon returned will be MRI. We know how it works on Windows so we test it this way. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 16:53:06 GMT, Alexander Zuev wrote: >> As for the example on Linux, how it will work for different sizes? >>Icon icon1 = fsv.getSystemIcon(new File("."), 16); >>Icon icon2 = fsv.getSystemIcon(new File("."), 32); >> Will the resulted icons have proper size 16 and 32? > > No they will have the same size. That's why the broad wording is used that we > take a requested size into consideration but we will return the best possible > icon we can get and we do not guarantee that the icon size will change the > outcome. Even on Windows if we request icon if sizes 1, 2, 3 and 4 the icon > will be basically the same - minimal quality icon available. > My point was that the implspec is a normative specification and we cannot > refer to non-public classes in that documentation. implSpec may describe the behavior of the default implementation and if it means referring the non-public API to clarify the behavior of this method i do not see any issue here. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 16:13:41 GMT, Sergey Bylokhov wrote: >> My point was that the implspec is a normative specification and we cannot >> refer to non-public classes in that documentation. > > As for the example on Linux, how it will work for different sizes? >Icon icon1 = fsv.getSystemIcon(new File("."), 16); >Icon icon2 = fsv.getSystemIcon(new File("."), 32); > Will the resulted icons have proper size 16 and 32? No they will have the same size. That's why the broad wording is used that we take a requested size into consideration but we will return the best possible icon we can get and we do not guarantee that the icon size will change the outcome. Even on Windows if we request icon if sizes 1, 2, 3 and 4 the icon will be basically the same - minimal quality icon available. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v7]
On Thu, 20 May 2021 16:43:05 GMT, Alexander Zuev wrote: >> Later we say that this method returns null for non-existed files. is it >> always correct? I am not sure that the file created for the library report >> true for the exists() method; DId we test this usecase? > > We do not test for that in the regression test but i did tested it manually > and we do return null for the non-existed files. I tested it on non-windows > platform too. We still return null. > Are we sure that all possible paths can be pointed by the file object? We specify that we return the icon for a file. If path can not be resolved in the file object we can not return the icon for it. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v7]
On Thu, 20 May 2021 16:06:37 GMT, Sergey Bylokhov wrote: >> Are we sure that all possible paths can be pointed by the file object? >> Especially some "Windows Libraries" which are accessed by the shell folder? > > Later we say that this method returns null for non-existed files. is it > always correct? I am not sure that the file created for the library report > true for the exists() method; DId we test this usecase? We do not test for that in the regression test but i did tested it manually and we do return null for the non-existed files. I tested it on non-windows platform too. We still return null. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8208747: [a11y] [macos] In Optionpane Demo, inside ComponentDialog Example, unable to navigate to all items, with VO on
On Thu, 20 May 2021 06:56:10 GMT, Pankaj Bansal wrote: > Is there a reason it is not causing an issue in case if JList? JList also has > elements inside a container, but I see that we can navigate the elements in > JList though there is also one issue that the VO always says "1 on n" (n is > total number of elements) on selecting any element of list. I am just > wondering why is this only needed for combobox and other components seem to > work fine. It is by far is not limited with JList. It also affects menu navigation with the accessibility shortcuts - in open menu with AS we can'[t walk past first item. It is particularly bad in JList because traversing trough JList causes a lot of events to be generated such as selection/deselection of the list elements, focus change and such. This causes OS to re-evaluate the current selection by asking either container or child about the current selection index and that is what generates so fierce special effect of multiple selection and selections changing because depending on the order in which system events got populated and developed we update selection in different (but always incorrect) order. - PR: https://git.openjdk.java.net/jdk/pull/4084
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 08:25:25 GMT, Sergey Bylokhov wrote: > The @implSpec is part of the specification, it is different from the > @implNote, no? > https://bugs.openjdk.java.net/browse/JDK-8266541?focusedCommentId=14419988=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14419988 > > If we will specify this method in a way that will require support on all > platforms we will get tck-red immediately after this push. Not exactly. The implNote is a note for future maintainers or people who will extend the functionality of the method. There will be no tck-red because the method is working and we did noted that we are taking into consideration the icon size and whenever technical possible we should return the multiresolution icon. So, for example, on Linux code `FileSystemView fsv = FileSystemView.getFileSystemView(); Icon icon = fsv.getSystemIcon(new File(".")); Icon icon2 = fsv.getSystemIcon(new File("."), 16); System.out.println("icon = " + icon); System.out.println("icon2 = " + icon2); ` will get icon and icon2 as the same single-resolution icon - but that will change when underlying implementation will be fixed. Right now it is not technical possible to return multi-resolution icon - we do not do it on Linux. Implementing the underlaying code for different system, as i said, is outside of the scope of this change. - PR: https://git.openjdk.java.net/jdk/pull/2875
Integrated: 8264290: Create implementation for NSAccessibilityComponentGroup protocol peer
On Mon, 17 May 2021 08:22:59 GMT, Alexander Zuev wrote: > Initial implementation. This pull request has now been integrated. Changeset: f07dcf47 Author: Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/f07dcf471c160e09fbc748124998923e7d453e66 Stats: 63 lines in 3 files changed: 60 ins; 0 del; 3 mod 8264290: Create implementation for NSAccessibilityComponentGroup protocol peer Reviewed-by: pbansal - PR: https://git.openjdk.java.net/jdk/pull/4046
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 08:18:02 GMT, Sergey Bylokhov wrote: >>> What are the "virtual pixels"? I remember we refer to something similar by >>> the point in the "user space coordinate system" Or probably we use the >>> virtual pixels somewhere? >> >> It is to say that the sizes are given in the same pixels as other components >> in the container and are subject to be rendered with different resolution >> based on the display scaling factor with preserving of relative sizes and >> proportions. The same terminology is used i.e. in SurfaceData. > > SurfaceData is not a public class, do we use this term somewhere in the spec? > If not then it will be better to use size/points in the user space coordinate > system, it is used already in the java2d. The CSR is already approved and changing specification at this point will require to roll it back to draft and then going trough the approval process again which in turn risks this change not to make it in the upcoming LTS release. I would prefer to integrate it and create a follow-up bug to clarify the wording where we can discuss the matter and - if it is worth it - to submit a new CSR to amend the specification. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 07:47:02 GMT, Sergey Bylokhov wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Empty tag before @implSpec causes warning during javadoc generation > > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 272: > >> 270: * returned is a multi-resolution icon image, >> 271: * which allows better support for High DPI environments >> 272: * with different scaling factors. > > Is the above text correct on all platforms? If it is not always MRI then how > the user should use the icon? instanceof+cast? BTW an example does not show > how to solve the bug itself, on how to access the "large icons". > > Need to clarify: the implSpec is a part of the specification so can we point > the non public "ShellFolder" class? implSpec marks that the paragraph below describes the details and logic of the default implementation and not the API specification. This tag also says that it can be changed in overriding or extending methods so it is Ok to specify non-public class to help describe the implementation specifics. As for the correctness on all platforms - that's the end goal of this new method and i believe it should be implemented this way everywhere where technically possible. But exact implementation on all platforms except Windows is outside of the scope of this exact changeset. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
On Thu, 20 May 2021 07:46:28 GMT, Sergey Bylokhov wrote: > What are the "virtual pixels"? I remember we refer to something similar by > the point in the "user space coordinate system" Or probably we use the > virtual pixels somewhere? It is to say that the sizes are given in the same pixels as other components in the container and are subject to be rendered with different resolution based on the display scaling factor with preserving of relative sizes and proportions. The same terminology is used i.e. in SurfaceData. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v10]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Empty tag before @implSpec causes warning during javadoc generation - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/59224696..09c7f8d7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=08-09 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8266949: Check possibility to disable OperationTimedOut on Unix [v2]
On Sun, 16 May 2021 01:29:59 GMT, Sergey Bylokhov wrote: >> Our implementation of realSync on Unix is not straightforward, we try to >> move the awt root window and catch the ConfigureNotify. The problem is that >> the request to move the awt root window can be ignored, and it is possible >> that the notification will not come. In this case, we throw non-specified >> OperationTimedOut and it crashes the random tests here and there. >> >> I have applied the same logic as on Windows and Linux we will wait till >> timeout(10 seconds) and then exit from the realsync. > > Sergey Bylokhov has updated the pull request incrementally with one > additional commit since the last revision: > > Update SunToolkit.java Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4038
Re: RFR: 8264290: Create implementation for NSAccessibilityComponentGroup protocol peer
On Tue, 18 May 2021 13:25:04 GMT, Pankaj Bansal wrote: > Could you please give some details about how you have tested this? I used the SwingSet2 first demo - internal frame - and made sure that components that were previously reported as groups - internal frame, button group and such - are correctly reported as groups and there are no regressions with navigating their children using the accessibility shortcut keys. Also with debug output i made sure that new code is used to gather list of the children inside the group components. Additionally i tested it with few existing AWT tests and made sure that there are no regression there too. - PR: https://git.openjdk.java.net/jdk/pull/4046
Re: RFR: 8208747: [a11y] [macos] In Optionpane Demo, inside ComponentDialog Example, unable to navigate to all items, with VO on
On Tue, 18 May 2021 09:30:28 GMT, Pankaj Bansal wrote: > The bug is a confidential issue. Either the bug should not be confidential or > this review should not be done here. I do not see any potential reason why this issue might be confidential. It is not security related, can be reproduced using the readily available demo and affects a large part of functionality. Removing the confidential label from the issue. - PR: https://git.openjdk.java.net/jdk/pull/4084
RFR: 8208747: [a11y] [macos] In Optionpane Demo, inside ComponentDialog Example, unable to navigate to all items, with VO on
Added accessibilityIndex function that correctly returns the index of a child in parent container - Commit messages: - 8208747: [a11y] [macos] In Optionpane Demo, inside ComponentDialog Example, unable to navigate to all items, with VO on Changes: https://git.openjdk.java.net/jdk/pull/4084/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4084=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8208747 Stats: 8 lines in 1 file changed: 8 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4084.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4084/head:pull/4084 PR: https://git.openjdk.java.net/jdk/pull/4084
Re: RFR: 8182043: Access to Windows Large Icons [v9]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Fixed documentation based on CSR review feedback - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/911bc70b..59224696 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=07-08 Stats: 14 lines in 1 file changed: 7 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
RFR: 8264290: Create implementation for NSAccessibilityComponentGroup protocol peer
Initial implementation. - Commit messages: - 8264290: Create implementation for NSAccessibilityComponentGroup protocol peer Changes: https://git.openjdk.java.net/jdk/pull/4046/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4046=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264290 Stats: 63 lines in 3 files changed: 60 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4046.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4046/head:pull/4046 PR: https://git.openjdk.java.net/jdk/pull/4046
Re: RFR: 8182043: Access to Windows Large Icons [v7]
On Sun, 16 May 2021 18:49:24 GMT, Alexander Zvegintsev wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Slight change of wording in javadoc >> Fixed Win32ShellFolder2.getSystemIcon scaling issue > > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 271: > >> 269: * Example: >> 270: * FileSystemView fsv = FileSystemView.getFileSystemView(); >> 271: * Icon icon = fsv.getSystemIcon("application.exe", 64); > > Shouldn't the first parameter be a File instance instead of String? > `Icon icon = fsv.getSystemIcon(new File("application.exe"), 64);` Good catch! Yes, fixed both here and in CSR. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v8]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Example in JavaDoc fixed - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/4cd5a508..911bc70b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=06-07 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v7]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Slight change of wording in javadoc Fixed Win32ShellFolder2.getSystemIcon scaling issue - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/10bae9be..4cd5a508 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=05-06 Stats: 5 lines in 2 files changed: 3 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v4]
On Fri, 14 May 2021 13:41:31 GMT, Alexey Ivanov wrote: >> I see - but still it has to be solved in the getShell32Icon method - which >> i'm going to do in a moment. > > `Win32ShellFolder2.getSystemIcon` is still affected, the return value should > be wrapped into MR-icon if its size is not equal to `LARGE_ICON_SIZE`. > > static Image getSystemIcon(SystemIcon iconType) { > long hIcon = getSystemIcon(iconType.getIconID()); > Image icon = makeIcon(hIcon); > if (LARGE_ICON_SIZE != icon.getWidth(null)) { > icon = new MultiResolutionIconImage(size, icon); > } > disposeIcon(hIcon); > return icon; > } Fixed. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8182043: Access to Windows Large Icons [v6]
On Fri, 14 May 2021 13:30:13 GMT, Alexey Ivanov wrote: >> Alexander Zuev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Make getShell32Icon method return multi-resolition image in case of >> requested icon size and actual icon size mismatch. > > src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java > line 266: > >> 264: * {@code ShellFolder} class. Whenever possible, the icon >> 265: * returned will be a multi-resolution icon image, >> 266: * which will allow better support for High DPI environments > > I'm not sure but probably “…which allows better…” is more grammatical, as > well as in the line above: “the icon returned is a multi-resolution”. > > Phil or Joe could correct this. Agree. - PR: https://git.openjdk.java.net/jdk/pull/2875
Re: RFR: 8264299: Create implementation of native accessibility peer for ScrollPane and ScrollBar Java Accessibility roles [v5]
On Thu, 13 May 2021 05:57:23 GMT, Pankaj Bansal wrote: >> Create implementation of native accessibility peer for ScrollPane and >> ScrollBar Java Accessibility roles. >> >> There are no direct protocols for these two roles and there is no protocol >> in list of role based protocols defined by Apple Accessibility API, which >> can be used for these roles directly. So ScrollAreaAccessibility and >> ScrollBarAccessibility classes do not implement any of the role based >> protocols in this list. >> https://developer.apple.com/library/archive/documentation/Accessibility/Conceptual/AccessibilityMacOSX/ImplementingAccessibilityforCustomControls.html. >> >> >> By looking at the current implementation of ScrollAreaAccessibility class in >> JavaComponentAccessibility and the complete list of properties and methods >> which can be used for any NSAccessibility class listed in >> https://developer.apple.com/documentation/appkit/nsaccessibility, custom >> implementation of ScrollAreaAccessibility and ScrollBarAccessibility is >> created. >> >> I have tested these implementations by running ScrollPane demo in SwingSet2 >> and scrollPane demo here >> https://docs.oracle.com/javase/tutorial/uiswing/components/scrollpane.html. >> The Voice Over behaviour is consistent with the old implementation and user >> can navigate and interact with the ScrollArea children hierarchy as expected. >> >> The old ScrollAreaAccessibility class in JavaComponentAccessibility is >> renamed to ScrollAreaAccessibility_Old as this was causing compiler errors >> and we are not deleting old implementations as of now. Please let me know if >> there is a better way to handle this. >> Also, some constants defined in JavaComponentAccessibility.m have been moved >> to JavaComponentAccessibility.h to avoid duplication in >> ScrollAreaAccessibility. > > Pankaj Bansal has updated the pull request incrementally with one additional > commit since the last revision: > > Remove old ScrollAreaAccessibility from JavaComponentAccessibility Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3794
Re: RFR: 8264299: Create implementation of native accessibility peer for ScrollPane and ScrollBar Java Accessibility roles [v3]
On Tue, 11 May 2021 06:44:26 GMT, Pankaj Bansal wrote: >> Create implementation of native accessibility peer for ScrollPane and >> ScrollBar Java Accessibility roles. >> >> There are no direct protocols for these two roles and there is no protocol >> in list of role based protocols defined by Apple Accessibility API, which >> can be used for these roles directly. So ScrollAreaAccessibility and >> ScrollBarAccessibility classes do not implement any of the role based >> protocols in this list. >> https://developer.apple.com/library/archive/documentation/Accessibility/Conceptual/AccessibilityMacOSX/ImplementingAccessibilityforCustomControls.html. >> >> >> By looking at the current implementation of ScrollAreaAccessibility class in >> JavaComponentAccessibility and the complete list of properties and methods >> which can be used for any NSAccessibility class listed in >> https://developer.apple.com/documentation/appkit/nsaccessibility, custom >> implementation of ScrollAreaAccessibility and ScrollBarAccessibility is >> created. >> >> I have tested these implementations by running ScrollPane demo in SwingSet2 >> and scrollPane demo here >> https://docs.oracle.com/javase/tutorial/uiswing/components/scrollpane.html. >> The Voice Over behaviour is consistent with the old implementation and user >> can navigate and interact with the ScrollArea children hierarchy as expected. >> >> The old ScrollAreaAccessibility class in JavaComponentAccessibility is >> renamed to ScrollAreaAccessibility_Old as this was causing compiler errors >> and we are not deleting old implementations as of now. Please let me know if >> there is a better way to handle this. >> Also, some constants defined in JavaComponentAccessibility.m have been moved >> to JavaComponentAccessibility.h to avoid duplication in >> ScrollAreaAccessibility. > > Pankaj Bansal 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 six additional > commits since the last revision: > > - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8264299 > - Use accessibilityRoleAttribute to get component role > - Add ScrollBar Accessibility > - remove logs > - Add functions to get ScrollBars from ScrollArea children > - 8264299: Create implementation of native accessibility peer for ScrollPane > Java Accessibility role Marked as reviewed by kizune (Reviewer). src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 1879: > 1877: > 1878: > 1879: @implementation ScrollAreaAccessibility_Old Why do we need to keep the old interface and implementation? When can it be instantiated? - PR: https://git.openjdk.java.net/jdk/pull/3794
Re: RFR: 8264299: Create implementation of native accessibility peer for ScrollPane and ScrollBar Java Accessibility roles [v3]
On Tue, 11 May 2021 06:44:26 GMT, Pankaj Bansal wrote: >> Create implementation of native accessibility peer for ScrollPane and >> ScrollBar Java Accessibility roles. >> >> There are no direct protocols for these two roles and there is no protocol >> in list of role based protocols defined by Apple Accessibility API, which >> can be used for these roles directly. So ScrollAreaAccessibility and >> ScrollBarAccessibility classes do not implement any of the role based >> protocols in this list. >> https://developer.apple.com/library/archive/documentation/Accessibility/Conceptual/AccessibilityMacOSX/ImplementingAccessibilityforCustomControls.html. >> >> >> By looking at the current implementation of ScrollAreaAccessibility class in >> JavaComponentAccessibility and the complete list of properties and methods >> which can be used for any NSAccessibility class listed in >> https://developer.apple.com/documentation/appkit/nsaccessibility, custom >> implementation of ScrollAreaAccessibility and ScrollBarAccessibility is >> created. >> >> I have tested these implementations by running ScrollPane demo in SwingSet2 >> and scrollPane demo here >> https://docs.oracle.com/javase/tutorial/uiswing/components/scrollpane.html. >> The Voice Over behaviour is consistent with the old implementation and user >> can navigate and interact with the ScrollArea children hierarchy as expected. >> >> The old ScrollAreaAccessibility class in JavaComponentAccessibility is >> renamed to ScrollAreaAccessibility_Old as this was causing compiler errors >> and we are not deleting old implementations as of now. Please let me know if >> there is a better way to handle this. >> Also, some constants defined in JavaComponentAccessibility.m have been moved >> to JavaComponentAccessibility.h to avoid duplication in >> ScrollAreaAccessibility. > > Pankaj Bansal 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 six additional > commits since the last revision: > > - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8264299 > - Use accessibilityRoleAttribute to get component role > - Add ScrollBar Accessibility > - remove logs > - Add functions to get ScrollBars from ScrollArea children > - 8264299: Create implementation of native accessibility peer for ScrollPane > Java Accessibility role src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 54: > 52: #define JAVA_AX_VISIBLE_CHILDREN (-3) > 53: // If the value is >=0, it's an index > 54: Since we are going to get rid of the JavaComponentAccessibility in the future i would rather copy the definitions into the CommonComponentAccessibility.h - that would save the double work in the future. - PR: https://git.openjdk.java.net/jdk/pull/3794
Re: RFR: 8182043: Access to Windows Large Icons [v6]
> Fix updated after first round of review. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Make getShell32Icon method return multi-resolition image in case of requested icon size and actual icon size mismatch. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2875/files - new: https://git.openjdk.java.net/jdk/pull/2875/files/a481b29b..10bae9be Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2875=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2875=04-05 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2875.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875 PR: https://git.openjdk.java.net/jdk/pull/2875