135395 - Add framework code for mozilla accessibility on Linux

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Bolian Yin]

Comment 1

[Bolian Yin]

see Mozilla Accessbility on Linux/Unix
(http://bugzilla.mozilla.org/attachment.cgi?id=77614&action=view) and bug 127893
for background information.

this bug should implement the framework of MAI (Mozilla ATK Implementation)
mentioned in the above document.

Comment 2

[Bolian Yin]

1. what's done
 (1) init process is done: If  Atk-bridge is loaded,  mozilla will init the
Accessibility support for ATK interfaces.  If Atk-bridge is missing, nothing
happen, mozilla run as normal without Atk support.
 (2) dependences is reasonable:
   Compile time:   Atk
   Run Time :    
    a. with accessibility support: all the stack of accessibility libs on Gnome2
    b. without  accessibility support : Atk libs
 (3)  Simple framework can work:  AT apps on the other side can get reference to
nsAccessible Object in Mozilla, and call one of its method , "GetName"

2. Main Problems:
 (1)  Tow ways of Implementing Atk support  to be chosen:
   a.  like mozilla on Windows:  on the same library of  widget/src/windows,  
loaded when mozilla startup.
   b.  like gail on Gnome:   seperate shared library, loaded when user set the
environment GTK_MODULES.  Then if user not config accessibility environment,  he
got zero overhead.
(2)  How to sent Atk sigals to Atk-bridge
  a. like mozilla on Windows:  One Heavy Atk Object sents all signals for all
nsAccessible object.
  b. like gail on Gnome:  Many Light  Atk Objects sent signal for  different
nsAccessible object.
    All of the  2x2 ways  can work, just tradeoff.  I am not very sure which one
is the best,  but personally I prefer "(1).b+(2).b".

Bill and Peter,  do you have any suggestion?   If you find I am on  the wrong
way, please pull me back as early as possible.

Ant Comments, questions and complaints are welcomed, as early as possible. 

Comment 3

[Jay Yan]

Peter Korn wrote:

I defer to Bill on this; but since you asked my opinion as well, here it is: 
I too vote for (1).b - only load the library when an environment variable is
set.  Note: it may not be GTK_MODULES in the future.  I believe a new
mechanism is being introduced/has been introduced that will superceed
GTK_MODULES.  I also lean toward (2).b.  We are used to having the various
Atk objects firing their own events.  We need to track ref-counts on them
anyway, so we don't save that task by going with (2).a.

Regards,

Peter

Comment 4

[Bolian Yin]

Attachment: draft frame code, Part 1: changes on existing files

Comment 5

[Bolian Yin]

Attachment: draft frame code, Part 2: new files for MAI

Comment 6

[Bolian Yin]

Attachment: draft frame code, Part 3: changes on Makefile.in

Comment 7

[Bolian Yin]

Attachment: draft frame code, tar.gz of the above 3 parts

Comment 8

[Bolian Yin]

The draft frame code pasted, many parts need to be changed later.

Comment 9

[Bolian Yin]

Attachment: a missing patch from above, sorry

Comment 10

[Bolian Yin]

Attachment: the all-in-one patch

The new one include all patches for all the changes needed, they are:
  1. new directory created: mozilla/widget/src/gtk2/mai
  2. new files add in the new directory (see above):
    nsMaiObject.h    nsMaiRoot.h	nsMaiUtil.h	   nsMaiWidget.h 
    nsMaiObject.cpp  nsMaiRoot.cpp	nsMaiUtil.cpp	   nsMaiWidget.cpp
    nsMaiTopLevel.h   nsMaiTopLevel.cpp   Makefile.in
  3. changed files in mozilla/widget/src/gtk2:
       nsWindow.h  nsWindow.cpp  nsCommonWidget.h  nsCommonWidget.cpp
       nsAppShell.cpp	Makefile.in
  4.  others configuration files changed:
     mozilla/allmakefiles.sh
     mozilla/widget/src/Makefile.in

Comment 11

[Jay Yan]

*******Jay Yan said:*****************

the task of the framework is to solve the problem of compiling
dependency(http://bugzilla.mozilla.org/attachment.cgi?id=77614&action=view#Compiling_dependency)
and running
dependency(http://bugzilla.mozilla.org/attachment.cgi?id=77614&action=view#Runing_dependency)
and write some example code. We wanted to put some code on widget/src/gtk2,
because it's compiling dependency is similar with gtk2 port. of course,  the
accessibility code will be seperated with the gtk2 widgets through having a new
subdirectory: widget/src/gtk2/mai. Bolian is now making a new patch for it.



**********Chris blizzard wrote:*************


Yeah, I've been asked to review that code.  In general so far the code looks OK
but I haven't looked at it in any depth.  I'll go into this a little more in the
review but we should probably move all of that into the gtk2 widget library
instead of making mai a seperate library. Unless there's a really good reason to
keep it seperate that I'm overlooking, of course.

Anyway, I'm more than happy to include the MAI code in the gtk2 widget code. 
That's probably where it should live. 


******Jay Yan wrote:********

in the first 5 patches, Bolian put code directly on /widget/src/gtk2, after that
we think perhaps seperate the MAI code into different library will enhance the
performance and reduce the dependency, so we created a new seperated directory
under gtk2: MAI and put the code in this new directory, the 6th patch is for
this solutuon.

if Chris thinks we had better put the code directly on widget/src/gtk2, we can
make another all-in-patch and review/sr two patcheds at the same time, and
discuss and compare two solution, at last checkin the better solution. 



(Add Shaver to CC list, hope that Shaver does not mind)

Comment 12

[Jay Yan]

about the bug assign, patch review, and super review:

Now I changed the bug owner to Bolian, beause Bolian is the author of the patch.

Actually I have reviewed this patch, but I do not think I am the right person to
give r= to this patch( at least now:-)), today, 8 developers in
browser-china-atf will have a meeting to review this patch. (we are very very
careful:-) )

Can [e-mail] give r= for this patch before Chris give sr=?  if
not, any idea? 

thanks everybody.

Comment 13

[browser China accessibility task force]

The reviewing result from [e-mail]

1.   ACCESSIBILITY directive should enclose header in nsWindow.h ?    
    in widget/src/windows they did that.  
    We can do without the directive,  But if someone complile without 
ACCESSIBILITY and mai files are all in a sperate directory not complied, 
 I am afraid the compiler can not find the header.  
    So leave it for now, until we find more compelling reason.
2.  about the two new methods added in "nsWindow" class.
   a).  mark them as "private"
   b).  move the declares to the end of the class
   c).  remove "virtual", I do not find any other class want to override it.
   d).  keep the return value type "PRBool", do not use "NS_IMETHOD", 
this is what widget/src/windows/nsWindow.h does
3. adjust the format of the declare of "mTopLevelAccessible"
4.  move the destruction work of access from nsWindow::~nsWindow to 
nsWindow::Destroy
5.  align the  second line of calling NativeCreate in nsWindow::Create 
(two places)
   I is my fault, not UltraEdit's    :(
 6.  Add Comments to nsWindow::DispatchAccessibleEvent
   BTW, the name is the same in widget/src/windows/nsWindow.cpp
 7.  exchange the two parts of "==" in nsWindow::DispatchAccessibleEvent 
and add parentheses( ?how to spell this word?)
 8.  nsAppShell.cpp, in the comments, change "proxy" to "implementation"
 9.  Use extern "C" { } to enclose "mai_init".   Keep unchange
     After I check the code, they should keep using the C++ naming 
convention. So Leave it unchanged.
 11.  Use "gtype" in all the MAI code if possilbe
    I will check the exceptions, and change them.
 12.  Remove the inheritance from "nsISupports"
    

Comment 14

[Bolian Yin]

Attachment: new patch

1. new files in widget/src/gtk2
     nsMaiHook.h
2. new dir:  widget/src/gtk2/mai
   new files in widget/src/gtk2/mai
     Makefile.in	 nsMaiObject.h	    nsMaiTopLevel.h  nsMaiWidget.h
     nsMaiInterface.cpp  nsMaiRoot.cpp	    nsMaiUtil.cpp
     nsMaiInterface.h	 nsMaiRoot.h	    nsMaiUtil.h
     nsMaiObject.cpp	 nsMaiTopLevel.cpp  nsMaiWidget.cpp 
3. changed files in mozilla/widget/src/gtk2:
       nsWindow.h  nsWindow.cpp  nsCommonWidget.h  nsCommonWidget.cpp
       Makefile.in
4. others configuration files changed:
     mozilla/allmakefiles.sh
     mozilla/widget/src/Makefile.in

======================
this patch make some changes to the previous one:
1. mai now is a seperate lib, which can be loaded by gtk_init when:
   GTK_MODULES=mai
2. some changes according to the previous code-review meeting (see Comment #13
)

Comment 15

[Pete Zha]

Comment on attachment 80762 [details] [diff] [review]
new patch

+  InitAccessibleEvent(event);
+  nsEventStatus status;
+  DispatchEvent(&event, status);
+  result = (nsEventStatus_eConsumeNoDefault ? PR_TRUE : PR_FALSE == status);
should be 
result = (nsEventStatus_eConsumeNoDefault == status) ? PR_TRUE : PR_FALSE;

+  void 	       CreateTopLevelAccessible();
+  PRBool	       DispatchAccessibleEvent(nsIAccessible** aAccessible);
We can use NS_IMETHOD_(PRBool) to define it
NS_IMETHOD(PRBool)    DispatchAccessibleEvent(nsIAccessible** aAccessible);

r=browser-china-atf with above modification

Comment 16

[Jay Yan]

Bolian and John, 
Since Chris has cleaned up gtk2 code today, please update your workspace,
prepare new patch with right style, add Pete's comments, and place the
modification of Makefile.in into right place. thanks
Jay

Comment 17

[Bolian Yin]

Attachment: re-formating, merge comments

Hi Blizzard, please sr:
changes include:
  1. re-formating on the suggestion of Blizzard (many changes)
  2. some modification to make Forte CC compile correctly
  3. merge Pete's comments
thanks all,
-- Bolian

Comment 18

[Bolian Yin]

changed files attachment (id=81371): 
1. new files in widget/src/gtk2
     nsMaiHook.h
2. new dir:  widget/src/gtk2/mai
   new files in widget/src/gtk2/mai
     Makefile.in	 nsMaiObject.h	    nsMaiTopLevel.h  nsMaiWidget.h
     nsMaiInterface.cpp  nsMaiRoot.cpp	    nsMaiUtil.cpp
     nsMaiInterface.h	 nsMaiRoot.h	    nsMaiUtil.h
     nsMaiObject.cpp	 nsMaiTopLevel.cpp  nsMaiWidget.cpp 
3. changed files in mozilla/widget/src/gtk2:
       nsWindow.h  nsWindow.cpp  nsCommonWidget.h  nsCommonWidget.cpp
       Makefile.in
4. others configuration files changed:
     mozilla/allmakefiles.sh

Comment 19

[Jay Yan]

Comment on attachment 81371 [details] [diff] [review]
re-formating, merge comments

verified these modificatons: based on new gtk2 trunk code,indent and bracket
format, makefile's location, and Pete's modification 
r=jay.yan

Comment 20

[Christopher Blizzard (:blizzard)]

Comment on attachment 81371 [details] [diff] [review]
re-formating, merge comments

need to fix some indenting in some of those files.  other than that sr=blizzard

Comment 21

[Bolian Yin]

Attachment: with indent fixed in some files

thanks everyone. This patch can be used for checkin

Comment 22

[Jay Yan]

Chris, 

now the tindexbox is red, it seems that I can not check it in before our long
holiday which is from May 1st to May7th. 
If you want to go on cleaning up gtk2 port code and do work based on this patch
nowadays, can you help to check them in? or I will recreat the patch according
to the trunk and check them in on May 8th. Thanks.
Jay

Comment 23

[Jay Yan]

checked in Bolian's new patch, which is based on new trunk.
and mark this bug as fixed.

Comment 24

[Jay Yan]

Comment on attachment 81638 [details] [diff] [review]
with indent fixed in some files

This patch was made on March 30th. today I checked in new patch.