If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

no enough arguments for ATK text-change and state-change events

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Kyle Yuan, Assigned: Kyle Yuan)

Tracking

Trunk
Sun
Solaris
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

15 years ago
needs to pass AtkStateChange/AtkTextChange data to the event handler.
(Assignee)

Updated

15 years ago
Blocks: 163832
Status: NEW → ASSIGNED
(Assignee)

Comment 1

15 years ago
Created attachment 98391 [details] [diff] [review]
fix nsRootAccessible::HandleEvent()

1) passed more arguments to mListener when EVENT_STATE_CHANGE and
EVENT_ATK_TEXT_CHANGE were fired;
2) remove EVENT_ATK_SELECTION_CHANGE event, because gnopernicus doesn't care
about that, it uses focus event instead;
3) fire a focus event when received "ListitemStateChange"
4) fire EVENT_REORDER when document was changed. at-poke need this event to
update its acc object tree.

All changes made within #ifdef MOZ_ACCESSIBILITY_ATK/#endif block

Seeking r=

Comment 2

15 years ago
Comment on attachment 98391 [details] [diff] [review]
fix nsRootAccessible::HandleEvent()

What do you mean, // currently,  we don't need to fire
EVENT_ATK_SELECTION_CHANGE here
Is everything based on Gnopernicus' needs?
Is Gnopernicus the only assistive technolgy you want to be compatible with?
Why not fire the event and let the assistive technology decide if it needs it?

-	 if (selectElement) // it's a HTML <select>
-	  
mListener->HandleEvent(nsIAccessibleEventListener::EVENT_ATK_SELECTION_CHANGE,
accessible, nsnull);
+	 if (selectElement) { // it's a HTML <select>
+	   // currently,  we don't need to fire EVENT_ATK_SELECTION_CHANGE here
+	 }

Is this going to change sometime in the future? 
Shouldn't we be calling some method on accessible text objects to get textData?
It doesn't look right.
+	 textData.start = 0;
+	 textData.length = 0;
+	 textData.add = PR_TRUE;

Above comment applies to childrenData.

This won't set the correct value for true (1). enable is a boolean.
If they check stateData.enable == TRUE, it's really comparing to 1.
stateData.enable = stateData.state & STATE_CHECKED;
I think you want
stateData.enable = (stateData.state & STATE_CHECKED) != 0

Don't you need to remove these lins from the "focus" handler, since tree items
get "select" now?
	  nsAccessibilityService::GetShellFromNode(targetNode,
getter_AddRefs(weakShell));
	  accessible = new nsXULTreeitemAccessible(accessible, targetNode,
weakShell, treeIndex);
Perhaps that code should be moved up in the method, next to
GetFocusedOptionNode(), so that it
doesn't need to be repeated in the 2 ATK and MSAA areas of this method.
Or, maybe we should combine that kind of stuff into 1 method,
GetEventAccessible() or something.
(Assignee)

Comment 3

15 years ago
> Why not fire the event and let the assistive technology decide if it needs it
Yes, Gnopernicus doesn't handle this kind of event yet. And in a near future, 
it's the only AT application in *nix platform. But I still can put the original 
code back for future use.

> Is this going to change sometime in the future? 
> Shouldn't we be calling some method on accessible text objects to get 
> textData?
> It doesn't look right.
It's hard to me to get which part of text was changed in that moment, because 
the content already changed. 

> Above comment applies to childrenData.
childrenData is different. childrenData.index = 0 and childrenData.child = 0 
means the all children were changed. Maybe I should assign -1 to 
childrenData.index.

> Don't you need to remove these lins from the "focus" handler, since tree items
> get "select" now?
Sorry for my low quality code. I'll simplify it.
(Assignee)

Comment 4

15 years ago
Created attachment 98556 [details] [diff] [review]
new patch per aaron's suggestion

changed per aaron's suggestion, reordered some code.

seeking r=.
Attachment #98391 - Attachment is obsolete: true
(Assignee)

Comment 5

15 years ago
Created attachment 98557 [details] [diff] [review]
sorry, should return NS_OK instead of NS_ERROR_FAILURE even if GetAccessibleFor failed
Attachment #98556 - Attachment is obsolete: true

Comment 6

15 years ago
Comment on attachment 98557 [details] [diff] [review]
sorry, should return NS_OK instead of NS_ERROR_FAILURE even if GetAccessibleFor failed

r=aaronl
Thanks Kyle.

Please add in a comment to explain this code, something like
// XXX kyle.yuan@sun.com uture work, put correct values for text change data

+      textData.start = 0;
+      textData.length = 0;
+      textData.add = PR_TRUE;

No new patch necessary for the comment.
Attachment #98557 - Flags: review+
Comment on attachment 98557 [details] [diff] [review]
sorry, should return NS_OK instead of NS_ERROR_FAILURE even if GetAccessibleFor failed

sr=jst
Attachment #98557 - Flags: superreview+
(Assignee)

Comment 8

15 years ago
checked in!
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.