Status

()

RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

Trunk
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Assignee)

Description

12 years ago
Created attachment 271062 [details] [diff] [review]
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?
Attachment #271062 - Flags: review?(aaronleventhal)

Comment 1

12 years ago
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+
(Assignee)

Comment 2

12 years ago
(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.
(Assignee)

Updated

12 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 3

12 years ago
gtk2 still use FireToolkitEvent.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 4

12 years ago
Created attachment 271093 [details] [diff] [review]
patc 2.1

add more methods into nsAccUtils to fire events
Attachment #271093 - Flags: review?(aaronleventhal)

Comment 5

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

Comment 6

12 years ago
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
Created attachment 271170 [details] [diff] [review]
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

12 years ago
Created attachment 271418 [details] [diff] [review]
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.
Attachment #271418 - Flags: review?(aaronleventhal)
(Assignee)

Comment 9

12 years ago
Created attachment 271801 [details] [diff] [review]
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
Attachment #271170 - Attachment is obsolete: true
Attachment #271418 - Attachment is obsolete: true
Attachment #271801 - Flags: review?(ginn.chen)
Attachment #271418 - Flags: review?(aaronleventhal)
(Assignee)

Updated

12 years ago
Duplicate of this bug: 387654

Updated

12 years ago
Attachment #271801 - Flags: review?(ginn.chen) → review+
(Assignee)

Comment 11

12 years ago
thank you for patches, checked in
(Assignee)

Comment 12

12 years ago
(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.
(Assignee)

Updated

11 years ago
Blocks: 389800
(Assignee)

Updated

10 years ago
Attachment #271093 - Flags: review?(aaronleventhal)
(Assignee)

Comment 13

10 years ago
Created attachment 345064 [details] [diff] [review]
patch
Attachment #271093 - Attachment is obsolete: true
Attachment #345064 - Flags: superreview?(benjamin)
Attachment #345064 - Flags: review?(aaronleventhal)
(Assignee)

Updated

10 years ago
Status: REOPENED → ASSIGNED

Comment 14

10 years ago
Comment on attachment 345064 [details] [diff] [review]
patch

rs=aaronlev
Attachment #345064 - Flags: review?(aaronleventhal) → review+
(Assignee)

Comment 15

10 years ago
Created attachment 345233 [details] [diff] [review]
patch2

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)

Updated

10 years ago
Attachment #345233 - Flags: superreview?(benjamin) → superreview+
(Assignee)

Comment 16

10 years ago
http://hg.mozilla.org/mozilla-central/rev/f2a93fd99987
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.