Comment from Joanie:

"NOT confirmed:

SUMMARY: 10 SUCCEEDED and 0 FAILED (0 UNEXPECTED) of 10 for
/home/jd/orca/test/keystrokes/gtk-demo/role_column_header.py

I wonder if this is related to the changes that were made in Gail w.r.t.
selection state. (I'm not convinced those made it into the separate Gail.)"

----

I'd tend to agree with this. I know I have a version of gail installed
which has those patches in.

But I have Gtk+ from trunk, so I *think* I have those fixes as well.  Any chance that Hardy overwrote your Gtk+ (or your Gail)?  If you applied the patches back when you were working on those bugs and updated hardy since then without reinstalling Gtk+ or Gail from trunk, my guess would be yes.

Joanie, what is the bug number for the gail changes?
I have a vague recollection that the changes were only
applied in one place, not both. I.e. in the gtk+/gail module
but not in the atk/gail module (or vica versa).

It's bug #499835.  The patch:
http://bugzilla.gnome.org/attachment.cgi?id=105010
has been applied to gtk/gtkiconview.c. 

But I seem to remember that the latest released version of
Gtk+ isn't using the gail code from the gtk+ module yet.
It's still using atk/gail, so doesn't a similar patch need 
to be have been applied there? I'm not sure that happened.

It's all way too confusing.

(In reply to comment #4)

> It's all way too confusing.

I take solace in the fact that *someone* understands it. :-)

All I know is that I have the latest Gtk+ and do not have the problem you do with the test.  My *guess* is that the significant difference between your system and mine is Gtk+.  (I have the latest at-spi from trunk, and of course the latest Orca, but everything else is current Hardy).

<shrugs>

> All I know is that I have the latest Gtk+ and do not have the problem you do
> with the test.

Okay, but which is right? Should we be saying "not selected" or not?

(In reply to comment #4)
> It's bug #499835.  The patch:
> http://bugzilla.gnome.org/attachment.cgi?id=105010
> has been applied to gtk/gtkiconview.c. 
> 
> But I seem to remember that the latest released version of
> Gtk+ isn't using the gail code from the gtk+ module yet.
> It's still using atk/gail, so doesn't a similar patch need 
> to be have been applied there? I'm not sure that happened.
> 
> It's all way too confusing.
> 

From Mattias Clasen:

Gnome 2.22 is going to use GTK+ 2.12, the latest release currently being
2.12.8 (I may or may not get another release out on Monday). The gail
merge is on trunk in svn, which will become GTK+ 2.14 sometime around
next Guadec. The icon view a11y fixes have been backported to 2.12.x and
are included in 2.12.8. 

GLib on the other hand, will get a new stable release in this Gnome
cycles, I will release 2.16 on Monday to go with Gnome 2.22.

What's (still) confusing to me is if there needed to be an atk/gail 
equivalent patch to the one that was applied for gtk/gtkiconview.c.
Do you know the answer to that Will? ;-)

(In reply to comment #8)
> What's (still) confusing to me is if there needed to be an atk/gail 
> equivalent patch to the one that was applied for gtk/gtkiconview.c.
> Do you know the answer to that Will? ;-)

We should all be able to determine this from the SVN history.  Hopefully I got this right, but you should double check just to make sure:

gtkiconview.c from gtk-2-12:
http://svn.gnome.org/viewvc/gtk%2B/branches/gtk-2-12/gtk/gtkiconview.c?r1=19539&r2=19538&pathrev=19539

gtkiconview.c from trunk:
http://svn.gnome.org/viewvc/gtk%2B/trunk/gtk/gtkiconview.c?r1=19538&r2=19537&pathrev=19538

So that is a "pure" Gtk+ change, not a gail one.

Then if the change is in gtk-2-12 and in SVN trunk, then
it can't be what's causing "not selected' to be spoken
for me and not for Joanie.

I'm also still not clear whether we want "not selected" 
spoken or not.

In other words, I'm still confused.

(In reply to comment #6)
 
> Okay, but which is right? Should we be saying "not selected" or not?

Not.

I often find it hard to look at test output -- and sometimes even the tests themselves -- and know what's right.  So in those cases I open up the test file and perform the same steps manually to see what on earth is going on and what we should be saying in response.  This is admittedly easier when you have dual-head 24-inch wide screen monitors. ;-)

For the failure in test 3, when I Down Arrow into the table, that first item is selected (I thought perhaps it might not be, similar to the tree store).  Since it's selected, we should not be saying that it's not. :-)

Tests 4, 7, and 8 are moving by cell rather than by row.  The whole business of saying "not selected" is to help the user understand what is taking place in a multi-select environment -- in particular, that one has given focus to an item that is not selected when one might expect that item to be selected.  In a table, this is only relevant when moving by row (i.e. Up/Down potentially change selection state; Left/Right don't).  Because tests 4, 7, and 8 are moving by cell and not by row, the announcement of selection state is irrelevant.  But let's pretend for a moment that it might be relevant. When I perform the test steps manually, the row (and hence the cells) in tests 4, 7, and 8 are selected.

Created attachment 107581 [details]
Orca debug output when running this regression test manually (upto test #3).

> For the failure in test 3, when I Down Arrow into the table, that first item is
> selected (I thought perhaps it might not be, similar to the tree store).  Since
> it's selected, we should not be saying that it's not. :-)

But that's the problem. For me it isn't selected and it's (quite rightly)
saying "not selected".

See attached debug output.

So which of us is correct? And why are we different?

Still confused. ;-)

> For the failure in test 3, when I Down Arrow into the table, that first item is
> selected (I thought perhaps it might not be, similar to the tree store).  Since
> it's selected, we should not be saying that it's not. :-)

Amendment to this statement.  We've got YASD (system difference).  :-( 

I just tried this on my old laptop, where I don't have the latest Gtk+.  On that box, pressing Down Arrow in test 3 causes the first item to have focus, but not to be selected.  So either they have changed Gtk+'s behavior or they have changed the demo's behavior.

Not sure what we want to do here.  I still think there is value in having the latest stuff so that we can catch regressions in the mainstream apps long before they get released.  By the same token, we'll have failures.

As a related aside, during Mike's CSUN presentation, Orca said "not selected" in ... I think it was nautilus ... at least once when it shouldn't have.  This was on Solaris.

> So which of us is correct? 

We both are. :-)

> And why are we different?

http://en.wikipedia.org/wiki/Dna
 
> Still confused. ;-)

Are you still still confused? ;-) Or are we at the point where:

1. We agree that the other three tests should not be causing 
   " not selected" to  be spoken

2. We are ready to discuss the YASD.

> 1. We agree that the other three tests should not be causing 
>    " not selected" to  be spoken

For me, moving left or right is not selecting those cells (if
they are not already selected). So we now need to determine whether
we should be saying "not selected" when moving left or right in
a table (if the cell isn't selected).

Is this written down in the spec anywhere?

Mike?

> 2. We are ready to discuss the YASD.

Sure.

Created attachment 107584 [details] [review]
Revision #1.

Assuming we don't want to say "not selected" when we are moving left
to right on a row, then this patch should fix that. With this patch,
just test #3 of gtk-demo/role_column_header.py fails (for me).

(In reply to comment #16)
> Assuming we don't want to say "not selected" when we are moving left
> to right on a row, 

Personally, I think that's the desired user interaction.  In a tree or tree table selection can only be changed for a row. Individual cells inherit the selection state of the row.  In other words, if a row is selected (or not), each cell in that row will be selected (or not).  Thus I think it's redundant for Orca to be speaking that information as one moves from item to item.  In a tree or tree table.

In an icon panel, however, where the icons are arranged in multiple rows and columns, selection can be changed when moving Up and Down AND when moving Left and Right.  I can move to a different column without selecting the icon in that column by pressing Control+Left/Right.  Therefore, in an icon panel, the (un)selected state is relevant for all directions.  And this patch causes it to not be spoken when moving Left and Right.

Created attachment 107618 [details] [review]
Alternate fix.

> Therefore, in an icon panel, the (un)selected state is relevant for all
> directions.  And this patch causes it to not be spoken when moving Left 
> and Right.

This revised patch would only reset checkIfSelected back to False
for table cells in tables or tree tables (if that is the desired
behavior).

Note though that for the "Icon View Basics" demo, when we Down arrow
into the icon pane and select the first icon, we get the following events:


KEYEVENT: type=0
          hw_code=104
          modifiers=0
          event_string=(Down)
          is_text=True
          time=1205938358.604479
orca.keyEcho: string to echo: Down
orca.isModifierKey: returning: False
orca.isNavigationKey: returning: True
orca.keyEcho: speaking: Down
SPEECH OUTPUT: 'Down'
orca.isModifierKey: returning: False
---------> QUEUEING EVENT focus:
DEQUEUED EVENT focus: <----------

vvvvv PROCESS OBJECT EVENT focus: vvvvv
OBJECT EVENT: focus:                                   detail=(0,0)
    app.name='gtk-demo' name='bin' role='icon' state='enabled focusable focused selectable sensitive showing visible' relations=''
LOCUS OF FOCUS: app='gtk-demo' name='bin' role='icon'
                event='focus:'
GENERATOR: _getDefaultBrailleRegions
           obj             = 
           role            = layered pane
GENERATOR: _getDefaultBrailleRegions
           obj             = 
           role            = scroll pane
GENERATOR: _getBrailleRegionsForFrame
           obj             = GtkIconView demo
           role            = frame
GENERATOR: _getDefaultBrailleRegions
           obj             = GtkIconView demo
           role            = frame
GENERATOR: _getDefaultBrailleRegions
           obj             = gtk-demo
           role            = application
GENERATOR: _getBrailleRegionsForIcon
           obj             = bin
           role            = icon
BRAILLE LINE:  'gtk-demo Application GtkIconView demo Frame ScrollPane LayeredPane bin Icon'
     VISIBLE:  'bin Icon', cursor=1
default.findCommonAncestor...
...default.findCommonAncestor
GENERATOR: _getSpeechForIcon
           obj             = bin
           role            = icon
           already_focused = False
           utterances:
               (bin)
               (icon)
SPEECH OUTPUT: ''
SPEECH OUTPUT: 'bin icon'
SPEECH OUTPUT: ' not selected'
^^^^^ PROCESS OBJECT EVENT focus: ^^^^^

---------> QUEUEING EVENT object:state-changed:focused
DEQUEUED EVENT object:state-changed:focused <----------

vvvvv PROCESS OBJECT EVENT object:state-changed:focused vvvvv
OBJECT EVENT: object:state-changed:focused             detail=(1,0)
    app.name='gtk-demo' name='bin' role='icon' state='enabled focusable focused selectable sensitive showing visible' relations=''
Finding top-level object for source.name=bin
--> obj.name=
--> obj.name=
--> obj.name=
--> obj.name=GtkIconView demo
^^^^^ PROCESS OBJECT EVENT object:state-changed:focused ^^^^^

---------> QUEUEING EVENT object:state-changed:selected
DEQUEUED EVENT object:state-changed:selected <----------

vvvvv PROCESS OBJECT EVENT object:state-changed:selected vvvvv
OBJECT EVENT: object:state-changed:selected            detail=(1,0)
    app.name='gtk-demo' name='bin' role='icon' state='enabled focusable focused selectable selected sensitive showing visible' relations=''
^^^^^ PROCESS OBJECT EVENT object:state-changed:selected ^^^^^

So it's correctly speaking "not selected" at the time of the "focus:"
because according to the icon state, it isn't selected. Then we get an
"object:state-changed:selected" which selects it, but we don't speak
anything.

We are failing to speak anything there because of the following
code in onStateChanged() in default.py (about line 3518):

                    if (keyString == "Down" or keyString == "Up") and \
                       event.source.getRole() == pyatspi.ROLE_TABLE_CELL and \
                       state.contains(pyatspi.STATE_SELECTED):
                            announceState = True

Should that be adjusted to also work with pyatspi.ROLE_ICON as well?

(In reply to comment #18)
> This revised patch would only reset checkIfSelected back to False
> for table cells in tables or tree tables (if that is the desired
> behavior).

Seems to work.  Thanks!
 
> Note though that for the "Icon View Basics" demo, when we Down arrow
> into the icon pane and select the first icon, we get the following events:

I'm not seeing this.  This is on my laptop with everything freshly installed.  Have at-spi and Orca from trunk; default hardy for everything else.  Things are working as I would expect using your current patch.

> Should that be adjusted to also work with pyatspi.ROLE_ICON as well?

If Orca is reliably speaking " not selected" in that case, I suppose we would want such an adjustment.  I'm still wondering what it is that is causing us to have different results on what I would think would be essentially the same configuration.

Thanks for your feedback Joanie.

Mike, we really need your input on this bug 
before we can progress it any further.

*** Bug 519841 has been marked as a duplicate of this bug. ***

*** Bug 525439 has been marked as a duplicate of this bug. ***

The "Alternate fix" patch has been committed to SVN trunk and the
gnome-2-22 branch. Moving to "[pending]".

Created attachment 108497 [details] [review]
Extra bit.

Needed to slightly adjust the original patch to make sure that
orca_state.lastNonModifierKeyEvent wasn't None. Patch committed
to SVN trunk and GNOME 2.22 branch.

The output here seems good. 

Thanks Mike. Closing as FIXED.