Created attachment 89151 [details]
Java program that displays a JTabbedPane with four tabs

It appears this bug is specific to the SwingSet2 demo. I've attached a simple test program that displays a JTabbedPane with four tabs. Flat review works properly with this test program.

It's possible that the flat-review logic cannot handle the complexity of SwingSet2. While this is still a bug, I recommend that its priority on the Orca task list be significantly lowered and a fix not committed to a GNOME release in the near future

Lynn -- do you know why there are problems with SwingSet2?  Is it giving an odd object hierarchy?  Is it something specific to SwingSet2 itself, or is the format of SwingSet2 going to be something that will occur in the majority of real world Java applications?

The hierarchy looks okay to me:

frame
  root pane
    panel
      layered pane
        panel
          panel
            panel
              panel
                tool bar
            page tab list
              page tab
              page tab
    menu bar

Going line-by-line from the top, flat-review, Orca visits 

1. menu bar
2. tool bar
3. filler
4. tabbed page panel

Maybe the flat-review code doesn't handle the page tab list being under layers of 'fillers'.

(In reply to comment #3)
> The hierarchy looks okay to me:
> 
> frame
>   root pane
>     panel
>       layered pane
>         panel
>           panel
>             panel
>               panel
>                 tool bar
>             page tab list
>               page tab
>               page tab
>     menu bar
> 
> Going line-by-line from the top, flat-review, Orca visits 
> 
> 1. menu bar
> 2. tool bar
> 3. filler
> 4. tabbed page panel
> 
> Maybe the flat-review code doesn't handle the page tab list being under layers
> of 'fillers'.

Interesting.  I wonder why 'filler' doesn't show up in the hierarchy you list above.  When running SwingSet2 and Orca, can you do an Insert+F8 and attach the results here?  Insert+F8 should print the hierarchy.

> 

Created attachment 89204 [details]
SwingSet2 hierarchy

'filler' shows up in the Insert-F8 hierarchy I attached. 'filler' probably didn't show up in the previous hierarchy, because at-poke filtered them out.

Hmmm...this looks like the page tabs are just being skipped and that flat review is speaking filler for something else.  I wonder - do Java page tabs implement the accessible text specialization?  They should, but this might be a limitation of the Java implementation.  If they do not, it seems as though getZonesFromAccessible would skip them as the result of the 'if' that starts around line 828:

        if (accessible.role != rolenames.ROLE_COMBO_BOX) \
            and (accessible.role != rolenames.ROLE_EMBEDDED) \
            and (accessible.role != rolenames.ROLE_LABEL) \
            and (accessible.role != rolenames.ROLE_MENU) \
            and accessible.childCount > 0:
            pass

I wonder if this should be something like:

        if not (accessible.role in [rolenames.ROLE_COMBO_BOX,
                                              rolenames.ROLE_EMBEDDED,
                                              rolenames.ROLE_LABEL,
                                              rolenames.ROLE_MENU,
                                              rolenames.ROLE_PAGE_TAB]) \
            and accessible.childCount > 0:
 

I'll give this a try. I think we need a general solution for Swing components.

1. All components that extend JTextComponent also implement AccessibleText

2. For components that do not extend JTextComponent, the components implement AccessibleText is two cases:

  a. the text is editable
  
  b. the text contains HTML

For editable JComboBoxes, a ComboBoxEditor is set to paint and edit the text area. This is typically a JTextField, but the implementation depends on the look and field.

For non-editiable JComboBoxes, a JLabel is used to render the text area, which does not implement AccessibleText.

So, the general principle is that non-editable text only implements AccessibleText if it is HTML. This holds for components that extend AbstractButton (JButton, JMenuItem, JToggleButton, etc.). Here's the getAccessibleText method for JLabel.

        public AccessibleText getAccessibleText() {
            View view = (View)JLabel.this.getClientProperty("html");
            if (view != null) {
                return this;
            } else {
                return null;
            }
        }

Are there other areas in Orca that make the (possibly false) assumption that all components containing text also implement AccessibleText (i.e., have a non-null 'text' field)? 

In general, if it paints text on the screen, it should support AccessibleText.  If it doesn't, it's a very unfortunate limitation of the implementation.

> Are there other areas in Orca that make the (possibly false) assumption that
> all components containing text also implement AccessibleText (i.e., have a
> non-null 'text' field)? 

In general, I think we try to support implementations that don't give us AccessibleText when they should.  

The particular area of flat review code from comment 6 might continue to be an area of concern.  Rather than do an exhaustive study, however, I'd feel comfortable with squashing specific bugs as we find out about them. That is, unless you already have an excellent suggestion in mind.

There is no general solution for Swing components that display text, but do not implement the AccessibleText interface. Peter and I discussed this at length with the Swing team. The underlying problem is that such text is rendered by the specific look and feel implementation, and is therefore look and feel dependent. The stumbling block is that there is no look and feel independent way of getting character bounds, and the Swing team opposes exposing look and feel implementation details through the Accessibility API. 

So, I agree with squashing specific bugs and I'll try your suggested fix.

Created attachment 89343 [details] [review]
proposed patch

The change proposed in Comment #4 fixes the problem. Flat-review correctly visits the SwingSet2 page tabs. I also tested with the TabbedPaneDemo previously attached and flat-review continues to work correctly.

(In reply to comment #10)
> Created an attachment (id=89343) [edit]
> proposed patch
> 
> The change proposed in Comment #4 fixes the problem. Flat-review correctly
> visits the SwingSet2 page tabs. I also tested with the TabbedPaneDemo
> previously attached and flat-review continues to work correctly.

Looks good.  Can you please test this with some GTK+ apps as well?  For example, the tabbed page of gnome-terminal's Edit->Current Profile... dialog?  If it works there, I say commit and close.  Thanks!

Tested with gnome-terminal Edit->Current Profile and found no regressions. Committing and closing as fixed.