Created attachment 53188 [details] [review]
patch to add these features to the IDL definition

Includes changes to various interfaces, and new interfaces.  In particular:

Accessibility_Terminal.idl: support for 2d text arrays, i.e. virtual terminals
Accessibility_Collection.idl: support for querying collections according to
various logical conditions and in varying sort orders - particularly useful for
complex documents
Accessibility_Document.idl: marker interface indicating that the elements below
this node in the tree are 'content' as opposed to user agent.  Also includes
some other useful API for instance locale, etc.

You add 3 methods to Accessible, and remove 2 unimplemented slots :-)
Of course, that changes the C ABI for the (unused & should not be exposed as
public/stable ABI anyway) libspi.

The Collection interface is interesting. IMHO the '_REVERSE_' variants in the
SortOrder look cumbersome & unpleasant. I would prefer a 'getPreviousChildren'
API call instead of that. getPreviousChildren(SORT_ORDER_REVERSE) type stuff
sounds pretty unpleasant.

Wrt. 'AccessibleSet' - I would be very inclined to use:

struct AccessibleSetEntry {
    Role aRole;
    string Name;
    Accessible theAccessible;
    Any expansion;
};
typedef sequence<AccessibleSetEntry> AccessibleSet;

Or somesuch - so that we can shove lots more data of the kind that people want
across the API.

Of course - this sort of thing comes down (again) to whether you think the CORBA
IDL should be used directly programatticaly - IMHO not a good idea :-)

For 'Document' - I'm inclined to believe that people can cope with not having
':'s in their attribute names - I'm less able to believe that they will
correctly (cross-language) escape ':'s in attribute values.

ie. I think it's better to define: [alpha][alphanum]*[colon].* as a attribute
parser - ie. require no escaping of the value at all.

To make that fly; it's only necessary to make Document::getAttributes() to
return a sequence<string> instead of a 1-massive-string thing.

The 'Command' API is somewhat worrying - what is this 'long id' thing in Command
? - that looks bad/scary to me and/or needs documenting properly.

Anyhow - apart from that looks interesting / intriguing :-)

HTH.

1) removal of too few slots was an error. Thanks for catching it, I certainly
intend to keep ABI compatible.

2) the reason for SORT_ORDER_REVERSE is that you sometimes want the "last 10"
elements.  If the Collection in question is large, you need this call for
performance reasons.

3) AccessibleSetEntry is interesting - I would shy away from the Any, since that
has been cumbersome in the past, but agree that Role and Name are very likely to
be of interest in all cases.  I am inclined to agree that AccessibleSet should
be a sequence of AccessibleSetEntry instead of just a sequence of Accessibles.

4) Agreed about Document::getAttributes.

5) the long id in Command is only for purposes of disambiguating Commands with
equal strings.  It happens in GOK for instance, that there can be multiple
Commands with the same name, but the order is exposed to the end-user and the
end-user knows from context which is which.  Ideally it's redundant info, but in
 reality ambiguous cases will crop up from time to time without it.

BTW thanks a lot Michael for your comments.

I still think the programmatic use of IDL is important for support of multiple
language/runtime environments.  We may move to a different model someday, but
that would involve specifying/certifying each language binding separately, which
has its own downside.

one other question regarding "...REVERSE"; the same could be accomplished in
three ways (AFAICS):

1) REVERSE variants of each SORT_ORDER (the currently proposed approach);
2) a "REVERSE" boolean parameter to the getChildren / getNextCHildren /
getPreviousChildren calls;
3) separate calls for reverse order variants of the above.

#2 adds a boolean param, is that nicer?  Not sure.
#3 is bad IMO because it doubles the number of new methods from 3 to 6; there is
a school of thought that doesn't mind method proliferation but I don't subscribe.
We already have getPrevChildren() which serves a similar purpose but does not
reverse the sort order.

Created attachment 53285 [details] [review]
patch, incorporating many of Michael's suggestions

revised patch:
1) fixed bincompat of Accessible
2) returns AttributeSet from Document, instead of doubly-delimited string;
3) defines AccessibleSet as sequence of AccessibleSetEntry structs

See also: 

http://accessibility.freestandards.org/~gk4/a11y/atspi-101005-bill/

Hi Bill.

* Other things we might want to add: an 'Accessible application' field in ~every
event emission - for event streaming.

* wrt. bincompat - I'm convinced it's a bad idea to support 'libspi' wrt.
bincompat which is about the only thing you break by expanding structs / interfaces.

* REVERSE: I don't understand the issue - but on the face of it it seems that
all REVERSE does is switch the semantics of 'getNext' 'getPrevious' - is that
the case ? Of course - if we want to get the 'last N' children - we could always
specify that a negative count to getChildren would go from the end.

IMHO adding this extra REVERSE level of complexity to the interface [ and
doubling the size of the enumeration ] will only create a mess that will have to
be elided in some way in the impl. anyway.

Failing that - can we have a bit-field for the enum:

const long REVERSE = 0x8000
const long <other guys>

So that the intention is clearer ?

Thanks Michael;

* application field in events - yes;
* libspi is gnome platform, thus we must maintain bincompat 
* REVERSE does not switch semantics of getNext/getPrevious.  getNext and
getPrevious both return the same order, just a different subset.  REVERSE
actually changes the order.  It's particularly critical if I want the "last n
elements" that match a given criterion, as opposed to the first.  

REVERSE is thus entirely necessary.  

I believe that bit-fields are a problem for IDL and for language bindings in
general.

Bill

most of the patch (minus Collection, Terminal, and Document) has now been committed to CVS HEAD.

Apologies for spam... ensuring Sun a11y folks are cc'ed on all current accessibility bugs.

Document is now in CVS HEAD.

We're really only missing Collection and Terminal now (and some minor enhancements to Table, Text, EditableText).

The dependencies are backwards in this bug report - this bug doesn't block the others, but the other way 'round.

Should we really keep this "tracker" bug around?  IMO we can find the relevant issues still open by searching for open RFEs against at-spi...

I'm okay with closing this tracker bug.

OK - trimming the summary and closing as Fixed, with the caveat that some aspects of this original tracker/rfe are not yet fixed (however they have separate open RFEs and are thus still being tracked).