<Swing Dev> Accessibility inner classes in AccessibleJTable do not stick to AccessibleComponent.getLocationOnScreen api

Pavel Porvatov pavel.porvatov at oracle.com
Mon Aug 6 13:54:47 UTC 2012


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