Too many warnings in console for accessibility!

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
12 years ago
8 years ago

People

(Reporter: aaronlev, Assigned: surkov)

Tracking

(Blocks 1 bug, {access})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [BK1])

Attachments

(2 attachments)

When I run with Inspect or another accessibility tool on Windows, I get a lot of useless error spew.
Attachment #261824 - Flags: review?(surkov.alexander)
Attachment #261824 - Flags: review?(surkov.alexander) → review+
Lets leave this open and go through the remaing warnings before we ship Firefox 3.

We should also check gcc warnings.
Blocks: keya11y
Surkov, nsAccessible::GetAttributes() spews a lot of messages here:
  NS_ENSURE_TRUE(attributes, NS_ERROR_FAILURE);
Assignee: aaronleventhal → surkov.alexander
gcc warnings of a11y module should be gone after checkin of bug 382296, bug 382293.

For debug console warnings,
There're a lot of NS_ENSURE_TRUE(accWrap, nsnull) should be changed to
if (!accWrap) return nsnull;
because accWrap could be null if it's shutdown
Posted patch patchSplinter Review
1) For MAI callbacks,
change NS_ENSURE_TRUE(accWrap, XXX) to if (!accWrap) return XXX;
We don't need these warnings

2) Merge GetAccessibleWrap and CheckMaiAtkObject
kill console warnings

3) Implement static method GetAtkObject(nsIAccessible *) as surkov's suggest, see bug 382567 comment 3

4) nsAccessibleWrap::FireAtkPropChangedEvent doesn't look correct to me.
I removed some code from it. Anyway, this method will never be called.

5) GetRowDescription is not implemented, so remove the code of getRowDescriptionCB
kill console warnings

6) small logic changes in getNameCB, getDescriptionCB, we don't care empty string here.
Attachment #267133 - Flags: review?(surkov.alexander)
OS: Windows XP → All
Attachment #267133 - Flags: review?(aaronleventhal)
Comment on attachment 267133 [details] [diff] [review]
patch

Having Surkov review should be enough.
Attachment #267133 - Flags: review?(aaronleventhal)
Comment on attachment 267133 [details] [diff] [review]
patch

Sorry for delay in review. I have few comments.

> AtkRole
> getRoleCB(AtkObject *aAtkObj)
>+        NS_ASSERTION(nsAccessible::IsTextInterfaceSupportCorrect(accWrap), "Does not support nsIAccessibleText when it should");

nit: is there offset here?

> AtkObject *
> getParentCB(AtkObject *aAtkObj)
>+    if (!aAtkObj->accessible_parent) {

Is it possible accessible parent has been changed but aAtkObj->accessible_parent holds old value?

nsAccessibleWrap::FireAtkPropChangedEvent(nsIAccessibleEvent *aEvent,
>                                           AtkObject *aObject)

>+    //Perhaps need more cases in the future

I would suggest to do not remove the code. This is needed for ATK and IA2 awaits too something similar. We need to file foollowing up bug to support them in gecko.

>Index: nsMaiInterfaceTable.cpp
> const gchar*
> getRowDescriptionCB(AtkTable *aTable, gint aRow)
> {

I would suggest to do not remove this code because I guess it's ok to fail if method is not implementd. We should rather to implement this instead to remove ATK mapping of the method.
Attachment #267133 - Flags: review?(surkov.alexander) → review+
Patch committed.

> Is it possible accessible parent has been changed but
> aAtkObj->accessible_parent holds old value?

I don't think accessible parent could be changed.
I didn't change code logic here.

I leave FireAtkPropChangedEvent and getRowDescriptionCB alone.
Keep in mind, the code in FireAtkPropChangedEvent is probably wrong.
And we don't need this function for general properties, e.g. "accessible-name"
atk_object_set_name already does it for us.
Blocks: cleana11y
No longer blocks: keya11y
Closing WFM since we got rid of warnings as the module was developed further.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Whiteboard: [BK1]
(In reply to Marco Zehe (:MarcoZ) from comment #9)
> Closing WFM since we got rid of warnings as the module was developed further.

we did that but I'm not sure we don't have them a lot anymore. Marco did you have a change to check the console?
You need to log in before you can comment on or make changes to this bug.