Accessibility inner classes in AccessibleJTable do not stick to AccessibleComponent.getLocationOnScreen api (original) (raw)

Frank Ding dingxmin at linux.vnet.ibm.com
Fri Aug 10 07:13:11 UTC 2012

Hi Pavel, Thanks for your comments from which I learned a lot. A new review patch is created by incorporating your comments @

http://cr.openjdk.java.net/~dingxmin/7188612/webrev.00/

Could you please review it again?

Best regards, Frank

On 8/6/2012 9:54 PM, Pavel Porvatov wrote:

Hi Frank,

The fix looks good but I have several comments for the test: 1. The Swing components should be created and accessed only from EDT thread. Therefore the assertGetLocation method should be on EDT as well 2. I'd remove lines 59-61 3. The lines 79 // Display the window. 80 frame.setVisible(true); 81 // make the frame invisible to test getLocationOnScreen() of 82 // JTable$AccessibleJTable$AccessibleJTableHeaderCell 83 // and JTable$AccessibleJTable$AccessibleJTableCell 84 frame.setVisible(false); don't guaranty that the frame becomes visible for awhile. You should use SunToolkit.realsync for that (take a look at other tests). Is it really necessary for the test to show and hide the frame? 4. Why don't you cast getAccessibleContext into AccessibleTable in one line, but wrote six lines for that: 88 AccessibleContext context = (AccessibleContext) table 89 .getAccessibleContext(); 90 // To get JTable$AccessibleJTable$AccessibleJTableHeaderCell instance, 91 // we need to first 92 // convert the context var to type AccessibleTable 93 AccessibleTable accessibleTable = (AccessibleTable) context; 5. Could you please remove useless comments like: 79 // Display the window. 80 frame.setVisible(true); 90 // To get JTable$AccessibleJTable$AccessibleJTableHeaderCell instance, 91 // we need to first 92 // convert the context var to type AccessibleTable 94 // and then get JTable$AccessibleJTable$AccessibleTableHeader etc. Only comments like 81 // make the frame invisible to test getLocationOnScreen() of 82 // JTable$AccessibleJTable$AccessibleJTableHeaderCell 83 // and JTable$AccessibleJTable$AccessibleJTableCell 100 // getLocation() must be null according to its javadoc and no exception 101 // is thrown looks reasonable for me in the provided test Regards, Pavel

Hi guys According to javadoc of AccessibleComponent.getLocationOnScreen, it returns null instead of throwing IllegalComponentStateException when the corresponding GUI object is not visible. However, two accessibility inner classes of JTable, JTable$AccessibleJTable$AccessibleJTableCell and Table$AccessibleJTable$AccessibleJTableHeaderCell implementing AccessibleComponent throw IllegalComponentStateException when the corresponding JTable is not visible. This behavior conflicts with the spec. I have submitted a bug in sunbug system reporting the issue, and received two acknowledgement emails (bug id 7188613 and 7188612, don't know why there are two?). I created a patch to fix the issue with a jtreg test case. Both the patch and the test are uploaded into the following space. http://cr.openjdk.java.net/~youdwei/ojdk-491/webrev.02/ Would anyone help to review the patch? Best regards, Frank

More information about the swing-dev mailing list