RFR: 8283214: [macos] Screen magnifier does not show the magnified text for JcomboBox [v9]
Alexey Ivanov
aivanov at openjdk.org
Thu Nov 9 15:54:29 UTC 2023
On Thu, 9 Nov 2023 04:39:32 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:
>> The issue exist only for non-editable combobox and the root cause is accessible object is not created due to incorrect index returned from component class which results in no a11y API invoked.
>>
>> Proposed solution is to return the correct accessible child from getAccessibleChild method which is AquaComboBoxButton (arrowButton) instance and that results in invoking the a11y APIs to return the current selected item in combobox.
>>
>> Further when the application comes up first time the accessible name is not set for current displayed item in JCombobox that is handled in AquaComboBoxButton which will take care for the current selected item as well as if user modifies the selection by drop-down list.
>>
>> CI link is posted in JBS.
>
> Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix for custom renderer
Changes requested by aivanov (Reviewer).
src/java.desktop/macosx/classes/com/apple/laf/AquaComboBoxButton.java line 201:
> 199: }
> 200:
> 201: final Component getRendererComponent() {
Suggestion:
private Component getRendererComponent() {
I wanted to use this method in `AquaComboBoxUI` but eventually I added `updateAccessibleName` instead, thus this method can be made `private`.
src/java.desktop/macosx/classes/com/apple/laf/AquaComboBoxUI.java line 143:
> 141: public void itemStateChanged(final ItemEvent e) {
> 142:
> 143: if (e.getStateChange() != ItemEvent.SELECTED) return;
Suggestion:
public void itemStateChanged(final ItemEvent e) {
if (e.getStateChange() != ItemEvent.SELECTED) return;
The blank line at the start of the method isn't necessary, is it?
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 1:
> 1: /*
You can use HTML for instructions and builder pattern for configuring `PassFailJFrame` if you like. I'm just advertising new features.
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 31:
> 29: * @requires (os.family == "mac")
> 30: * @summary Verifies if JComboBox selected item magnifies using
> 31: * screen magnifier a11y tool.
Suggestion:
* @summary Verifies if item selected in JComboBox magnifies using
* screen magnifier a11y tool
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 48:
> 46: private static JFrame frame;
> 47: private static final String INSTRUCTIONS =
> 48: "1) Enable Screen magnifier on the Mac \n\n" +
Suggestion:
"1) Enable Screen magnifier on the Mac\n\n" +
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 52:
> 50: "Select \"Enable Hover Text\"\n\n" +
> 51: "2) Move the mouse over the combobox selected item by pressing " +
> 52: "\"command\" button.\n\n" +
Suggestion:
"2) Move the mouse over the combo box and press " +
""Command" button.\n\n" +
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 53:
> 51: "2) Move the mouse over the combobox selected item by pressing " +
> 52: "\"command\" button.\n\n" +
> 53: "3) If magnified label is visible, Press Pass else Fail.";
Suggestion:
"3) If magnified label is visible, press Pass else Fail.";
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 68:
> 66:
> 67: String[] fruits = new String[] {"Apple", "Orange",
> 68: "Mango", "Pine Apple", "Banana"};
Suggestion:
"Mango", "Pineapple", "Banana"};
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 71:
> 69: JComboBox<String> comboBox = new JComboBox<String>(fruits);
> 70: JPanel fruitPanel = new JPanel(new GridLayout(1, 2));
> 71: JLabel fruitLabel = new JLabel("Fruits : ", JLabel.CENTER);
Suggestion:
JLabel fruitLabel = new JLabel("Fruits:", JLabel.CENTER);
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 75:
> 73: fruitPanel.add(fruitLabel);
> 74: fruitPanel.add(comboBox);
> 75: comboBox.getAccessibleContext().setAccessibleName("Fruit jCombobox");
Suggestion:
comboBox.getAccessibleContext().setAccessibleName("Fruit Combo box");
test/jdk/javax/accessibility/JComboBox/TestJComboBoxScreenMagnifier.java line 81:
> 79: PassFailJFrame.positionTestWindow(frame,
> 80: PassFailJFrame.Position.HORIZONTAL);
> 81: frame.setLocationRelativeTo(null);
Suggestion:
It's not needed, the location of the frame is set explicitly by the above call to `PassFailJFrame.positionTestWindow`.
test/jdk/javax/swing/JComboBox/6567433/UpdateUIRecursionTest.java line 1:
> 1: /*
You should update the copyright year.
I wonder if it makes sense to resolve warnings about raw classes.
-------------
PR Review: https://git.openjdk.org/jdk/pull/14497#pullrequestreview-1722830503
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388181016
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388182328
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388201084
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388185906
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388191437
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388193414
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388193775
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388190556
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388195249
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388188204
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388197459
PR Review Comment: https://git.openjdk.org/jdk/pull/14497#discussion_r1388204973
More information about the client-libs-dev
mailing list