Closed Bug 386978 Opened 17 years ago Closed 15 years ago

get rid FireToolkitEvent


(Core :: Disability Access APIs, defect)

Not set





(Reporter: surkov, Assigned: surkov)


(Blocks 1 open bug)


(Keywords: access)


(3 files, 4 obsolete files)

Attached patch patchSplinter Review
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?
Attachment #271062 - Flags: review?(aaronleventhal)
Comment on attachment 271062 [details] [diff] [review]

r+ as long as you tested it a little bit with screen readers or a11y tools.
Attachment #271062 - Flags: review?(aaronleventhal) → review+
(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.
Closed: 17 years ago
Resolution: --- → FIXED
gtk2 still use FireToolkitEvent.
Resolution: FIXED → ---
Attached patch patc 2.1 (obsolete) — Splinter Review
add more methods into nsAccUtils to fire events
Attachment #271093 - Flags: review?(aaronleventhal)
Why is it better to hide the work into nsAccUtils? I kind of like the way it is now.
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'
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*, 
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
X:/trunk/mozilla/accessible/src/html/nsHyperTextAccessible.h:70: error:         
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
FWIW, this is what I did to make accessible/src/mac/ build again.
You can probably cvs remove accessible/src/mac/ also.
Attached patch fix OS/2 build break (obsolete) — Splinter Review
Ah, so you forgot to remove nsDocAccessibleWrap.cpp from 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.
Attachment #271418 - Flags: review?(aaronleventhal)
Attached patch fix os/2 macSplinter Review
1) 271170: Some fixes for accessible/src/mac/
2) 271418: fix OS/2 build break
3) nsDocAccessileWrap.cpp/mm was removed from cvs
Attachment #271170 - Attachment is obsolete: true
Attachment #271418 - Attachment is obsolete: true
Attachment #271801 - Flags: review?(ginn.chen)
Attachment #271418 - Flags: review?(aaronleventhal)
Attachment #271801 - Flags: review?(ginn.chen) → review+
thank you for patches, checked in
(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.
Blocks: cleana11y
Attachment #271093 - Flags: review?(aaronleventhal)
Attached patch patch (obsolete) — Splinter Review
Attachment #271093 - Attachment is obsolete: true
Attachment #345064 - Flags: superreview?(benjamin)
Attachment #345064 - Flags: review?(aaronleventhal)
Comment on attachment 345064 [details] [diff] [review]

Attachment #345064 - Flags: review?(aaronleventhal) → review+
Attached patch patch2Splinter Review
Benjamin, it is clean up code of accessibility module.
Attachment #345064 - Attachment is obsolete: true
Attachment #345233 - Flags: superreview?(benjamin)
Attachment #345064 - Flags: superreview?(benjamin)
Attachment #345233 - Flags: superreview?(benjamin) → superreview+
Closed: 17 years ago15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.