Closed Bug 386978 Opened 16 years ago Closed 14 years ago

get rid FireToolkitEvent

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(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]
patch

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.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
gtk2 still use FireToolkitEvent.
Status: RESOLVED → REOPENED
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'
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
FWIW, this is what I did to make accessible/src/mac/ build again.
You can probably cvs remove accessible/src/mac/nsDocAccessibleWrap.mm also.
Attached patch fix OS/2 build break (obsolete) — Splinter Review
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.
Attachment #271418 - Flags: review?(aaronleventhal)
Attached patch fix os/2 macSplinter Review
patch
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)
Duplicate of this bug: 387654
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)
Status: REOPENED → ASSIGNED
Comment on attachment 345064 [details] [diff] [review]
patch

rs=aaronlev
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+
http://hg.mozilla.org/mozilla-central/rev/f2a93fd99987
Status: ASSIGNED → RESOLVED
Closed: 16 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.