386978 - get rid FireToolkitEvent

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

1) get rid FireToolkitEvent
2) use nsAccUtils to fire an accessible event. Reasons:

1) Do not create nsAccEvent directly (do not use FireAccessibleEvent) because nsAccEvent doesn't bring information whether the given event is delayed.
2) The method accessible->FireAccessibleEvent(accessibleEvent) has two accessibles. It may confuse.

Therefore I introduce helper method to fire an event. Later I'll introduce methods to fire delayed events and events of specific types.

Sounds ok?

Comment 1

[Aaron Leventhal]

Comment on attachment 271062 [details] [diff] [review]
patch

r+ as long as you tested it a little bit with screen readers or a11y tools.

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> (From update of attachment 271062 [details] [diff] [review])
> r+ as long as you tested it a little bit with screen readers or a11y tools.
> 

accevent shows events :)

checked in.

Comment 3

[alexander :surkov (:asurkov)]

gtk2 still use FireToolkitEvent.

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patc 2.1

add more methods into nsAccUtils to fire events

Comment 5

[Aaron Leventhal]

Why is it better to hide the work into nsAccUtils? I kind of like the way it is now.

Comment 6

[Peter Weilbacher]

Since attachment 271062 [details] [diff] [review] was checked in I get the error messages pasted below when compiling in accessible/src/other. This is with a SeaMonkey build on OS/2 -- I don't even know why we are compiling that directory on OS/2, but it has worked before, so I guess it should continue to work.

Backing out the typedef change to nsDocAccessibleWrap.h helps, but only to compile in that directory. When compiling in src/base later it fails again. I'm no C++ guru but using typedef for classes seems kind of weird to me...


make.exe[6]: Entering directory `X:/trunk/sm/accessible/src/other'
nsAccessNodeWrap.cpp
nsAccessibleWrap.cpp
nsDocAccessibleWrap.cpp
In file included from X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.h:46,
                 from X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:39:
X:/trunk/mozilla/accessible/src/html/nsHyperTextAccessible.h:74: warning: `
   virtual nsresult nsHyperTextAccessible::GetEditor(nsIEditor**)' was hidden
X:/trunk/mozilla/accessible/src/base/nsDocAccessible.h:128: warning:   by `
   virtual already_AddRefed<nsIEditor> nsDocAccessible::GetEditor()'
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:43: error: ISO 
   C++ forbids declaration of `nsDocAccessibleWrap' with no type
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:43: error: no `
   int nsDocAccessible::nsDocAccessibleWrap(nsIDOMNode*, nsIWeakReference*)' 
   member function declared in class `nsDocAccessible'
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp: In member 
   function `int nsDocAccessible::nsDocAccessibleWrap(nsIDOMNode*, 
   nsIWeakReference*)':
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:43: error: only 
   constructors take base initializers
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:44: error: type `
   class nsDocAccessible' is not a direct base of `nsDocAccessible'
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:45: error: no 
   matching function for call to `nsHyperTextAccessible::nsHyperTextAccessible(
   )'
X:/trunk/mozilla/accessible/src/html/nsHyperTextAccessible.h:68: error:
   candidates are: nsHyperTextAccessible::nsHyperTextAccessible(const
   nsHyperTextAccessible&)
X:/trunk/mozilla/accessible/src/html/nsHyperTextAccessible.h:70: error:         
           nsHyperTextAccessible::nsHyperTextAccessible(nsIDOMNode*, 
   nsIWeakReference*)
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp: At global scope:
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:49: error: 
   destructor `nsDocAccessibleWrap' must match class name `nsDocAccessible'
X:/trunk/mozilla/accessible/src/other/nsDocAccessibleWrap.cpp:53: error: no `
   nsresult nsDocAccessible::FireToolkitEvent(unsigned int, nsIAccessible*, 
   void*)' member function declared in class `nsDocAccessible'
make.exe[6]: *** [nsDocAccessibleWrap.o] Error 1
make.exe[6]: Leaving directory `X:/trunk/sm/accessible/src/other'
make.exe[5]: *** [libs] Error 2

Comment 7

[Mats Palmgren (inactive)]

Attachment: Some fixes for accessible/src/mac/

FWIW, this is what I did to make accessible/src/mac/ build again.
You can probably cvs remove accessible/src/mac/nsDocAccessibleWrap.mm also.

Comment 8

[Peter Weilbacher]

Attachment: fix OS/2 build break

Ah, so you forgot to remove nsDocAccessibleWrap.cpp from Makefile.in and CVS. If I do that and add Unload() to nsApplicationAccessibleWrap OS/2 builds again, too.
Is anything else influcenced by the stuff in src/other or should I just get this in to fix the OS/2 build break?
I guess that nsDocAccessibleWrap.cpp should then by CVS removed, too.

Comment 9

[alexander :surkov (:asurkov)]

Attachment: fix os/2 mac

patch
1) 271170: Some fixes for accessible/src/mac/
2) 271418: fix OS/2 build break
3) nsDocAccessileWrap.cpp/mm was removed from cvs

Comment 11

[alexander :surkov (:asurkov)]

thank you for patches, checked in

Comment 12

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> Why is it better to hide the work into nsAccUtils? I kind of like the way it is
> now.
> 

1) nsAccUtils saves the code lines (now we have 3-5 lines, nsAccUtils allows to have 1-2)
2) nsAccUtils hides the fact we have two accessibles to fire an event that confuses.
3) FireAccessibleEvent doesn't make a difference between delayed events and simple events. There is a chance to create delayed event instead of simple event and vice versa. nsAccUtils allows to avoid this.

nsAccUtils will provide clear interface to file events more easier and get rid potential errors.

Comment 13

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 14

[Aaron Leventhal]

Comment on attachment 345064 [details] [diff] [review]
patch

rs=aaronlev

Comment 15

[alexander :surkov (:asurkov)]

Attachment: patch2

Benjamin, it is clean up code of accessibility module.

Comment 16

[alexander :surkov (:asurkov)]

http://hg.mozilla.org/mozilla-central/rev/f2a93fd99987