127893 - Check in additional IDL files contributed by Sun

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[John Gaunt (redfive)]

There are about 8 accessibility IDL files to fill out our coverage for unix
accessibility. I'll post the initial drafts, they will need to be checked for
things like the right string classes and I know of one typo off hand. Also there
are a couple of methods that we know we won't need after some discussions with
the Sun folks.

Comment 1

[John Gaunt (redfive)]

Attachment: 8 Accessibility IDL files to be added

One file containing the 8 idl files we received from sun verbatim. I'll be
cleaning  them up and posting the revised files here.

Comment 2

[timeless]

        void getDescription(in long i,
                out string description);

should be string getDescription(in long i); and i'm not sure if get is correct.

        void selectAttributes (in long startPos,
                in long endPos,
                in nsISupports attributes);
nsISupports? no.

nsIAccessibleEditableText why are all of the methods void?

please get someone to write javadoc comments for all of these methods.

please cahnge all almost javadoc comments to /** javadoc */

nsIAccessibleSelection
why is everything in reference to indices when all the comments describe 
'objects' or their 'child'ren?
    /*
     * Removes the specified child of the object from the object's selection.
     */
    void removeSelection (in long i);


        void getSelectedColumns (out unsigned long columnsSize
                [retval, array, size_is(columnsSize] out long columns);
this can't possibly pass an idl compiler, can it?

        long isSelected (in long row,
                in long column);
boolean?

warning about wstring. we're landing the nsAUTF8 stuff ... 

        nsISupports getAttributeRange (in long offset,
                out long rangeStartOffset,
                out long rangeEndOffset);
nsISupports :-(

        void getSelectionBounds (in long selectionNum, 
                out long startOffset,
                out long endOffset);
we don't have any range object that you could return?

Comment 3

[John Gaunt (redfive)]

pushing out, since we won't get approval to check this in before the 1.0 branch

Comment 4

[Jay Yan]

Brendan and Judd: adding you guys as cc's on this because the Sun accessibility
guys would really like to get these IDL files in soon, that is, 1.0 time frame.
 Is this possible in our currently frozen state?  This is a no-code contribution
that forms the basis of accessibility framework to come. -- scc

Comment 5

[Jay Yan]

Attachment: Architecture of Mozilla Accessibility on Linux(GNOME)

Comment 6

[Jay Yan]

Attachment: The Draft Documentation of Mozilla Accessibility on Linux

This is a draft documentation about Mozilla Accessibility, before it is put on
mozilla.org, I attatch it here for 2 aims:
1 Scc, Brandan and Judd can read the documentation, so that the idl files can
be reviewed, approved, and checked-in sooner.
2 Peter and Bill can take a look at it, and give me revision suggeation, so
that it can be put on mozilla.org sooner.

Scc, Brandan and Judd, can you spend some time to read the documentation and
then review the idl files? if there is any problem, we are ready here to
response. 

Thanks a lot
Jay

Comment 7

[John Gaunt (redfive)]

Attachment: Patch for adding the idl to Win and Linux -- need to get Mac XML files updated!!

This is the clean up of the above idl files and presentation as a patch. I'd
like the Sun team to take a look at these closely, I've made a couple changes
mainly dealing with changing void return values to booleans. I've also marked
several methods with XXX in a comment block and requested clarification of the
function prototype. Sometimes an explanation of what is going on is needed,
sometimes it is merely verification that the type or name is correct. 

Any comments you want to add in the form of Javadoc style documentation I will
gladly roll up into a new patch. In fact if you just want to apply this patch,
make the comment changes and repost that would be ideal. 

When I get verification that this IDL is acceptable by the Sun folks I will
start fishing for r/sr.

Comment 8

[Jay Yan]

Hi John:
We test this patch, it works fine so far. 
Thanks a lot for your correcting the syntax error and the 
coding style. It must have taken you a lot of time.

Writing JavaDoc is a good idea to explain these idl file, 
and it is also helpful for writing code to implement these
interfaces, we are going to do it. But I am afraid now we 
want to apply these idl files to trunk before these Java 
doc is finished, because it may take us some time to polish 
the docs.

Bolian and I read your correction and comments, if we repost
a new patch, some of your valuable comments will be deleted, 
it is what we do not want to do.

so we think the best way is to answer your comments in each 
idl file one by one( if we are sure:-) )


nsIAccessibleAction.idl 
  wstring getDescription(in long index)   
    we agree with you.
  wstring getKeyBinding(in long index)   
    we agree with you.
  
nsIAccessbileEditableText.idl
  void selectionAttributes(...) 
    this function should be changed into setAttributes(...)
  boolean setTextContents(in wstring text) 
    in ATK, accordingly function returns void, but for all 
    platforms, your idea is better. we will use yours.
  
nsIAccessibleHyperLink.idl

nsIAccessibleHyperText.idl

nsIAccessibleSelection.idl
  boolean selectionAllSelection();  
    if the object does not accept multiple selection, it 
    should return false. So we can chenge the void into boolean.
  
nsIAccessibleTable.idl
  nsIAccessible refAt(in long row, in long column)
    this function gets a cell. about the name, it origins from ATK.
    In order to support all platforms, we can change it into 
    cellRefAt, it is up to you.:-)
  long getIndexAt(in long row, in long column)
    this function gets a index.
  long isSelected(in long row, in long column)
    this is as same as refAt()
  
nsIAccessibleText.idl
  readonly caretOffset
  boolean setCaretOffset(in long offset)
    Change readwrite attribute into a readonly attribute and a set 
    function, it is OK for us and for all the other platforms.
  nsISupport getAttributeRange
    It origins from get_run_attribute() in atktext.h, it is used 
    to get attribute-list of one text object. we are also not very
    sure about it now, we will go on investigating it, fortunately,
    we will implement text and table later, so now we can check it
    in, if there is error, we will file bug against it and modify it.
    
  return type of some other functions are changed into boolean, it is OK.
  
nsIAccessibleValue.idl
  readonly attribute double currentValue
  boolean setCurrentValue(in double value);
    Change a readwrite attribute into a readonly attribute and a set 
    function, it is OK.


Thanks again, John

Jay & Bolian
  
  
  
       
  
  
  

  

Comment 9

[Jay Yan]

Attachment: Repost new patch based on John's work

John,
 
Today, I got time to recreated this patch according to comment #8.
Besides modifying the patch according to comment#8, I also corrected some tiny
mistakes, such as code line which are more than 80 characters, and lines end
with space.
In this patch, there is still a "XXX" in nsIAccessibleText.idl, I am not sure
about it yet now, I am investigating it through talking with Mark and Bill, and
we have enough time to correct it after we check it in and before we write code
to implement it. (text and table are the last two elements we are going to
finish).

Can you r= it now?

Thanks

Jay

Comment 10

[John Gaunt (redfive)]

Attachment: mac patch to the xml file, needs to be rolled into the main patch

Comment 11

[John Gaunt (redfive)]

Attachment: Complete patch, cleaned up and including the mac build magic

Comment 12

[John Gaunt (redfive)]

-> jay 

As I've basically just been doing review work on this, I'm going to let him
check it in and give my review.

Comment 13

[John Gaunt (redfive)]

Comment on attachment 79559 [details] [diff] [review]
Complete patch, cleaned up and including the mac build magic

r=jgaunt

Comment 14

[Johnny Stenback (:jst)]

Comment on attachment 79559 [details] [diff] [review]
Complete patch, cleaned up and including the mac build magic

Fix the inconsistent usage of space-before-opening-paren-after-method-name,
i.e.:

  int foo (...);

vs.:

  int foo(...);

I'd vote for loosing the spaces, but most methods in these interfaces have them
so if that's the prefered way here, then fine, but be consistent. If you want
to be consistent with most other mozilla code, then loose the spaces.

sr=jst with that change.

Comment 15

[Jay Yan]

Attachment: the final patch checked in (a little modification according to jst's suggestion)

Checked in!!!!!

Thanks everybody's hard work!

Comment 16

[Jay Yan]

mark it as fixed

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

+ Assumptions:
+
+ string is a UTF-8 or most likely ASCII
+ encoding.

shouldn't you use AUTF8String, then?

Comment 18

[Jay Yan]

Here comes the javadoc for API!
http://www.mozilla.org/projects/ui/accessibility/unix/nsIAccessibleLibrary/nsIAc
cessibleLibrary.html

Comment 19

[Jay Yan]

annoying bugzilla bug, I can not input long URL.

Go to 
http://www.mozilla.org/projects/ui/accessibility/unix/index.html
(this page has more content)

click the 3rd section "Accessibility API", 

then you can see the detailed spec for IDL.