Closed Bug 571184 Opened 10 years ago Closed 10 years ago

remove nsIAccessNode traversal methods

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

Attached patch patchSplinter Review
1) Remove nsIAccessNode traversal methods
2) Cache nsIAccessible objects
3) Remove unnecessary query interfaces from nsIAcessNode while we're getting accessible object from the cache
4) Change return values of Init() and Shutdown()
5) It shouldn't affect on ISimpleDOMNode since it doesn't use the cache to get accessnode object if there's no accessible object
Attachment #450315 - Flags: review?(marco.zehe)
Attachment #450315 - Flags: review?(bolterbugz)
6) virtual void Init() in mac/nsDocAccessibeWrap.h is fixed locally
7) test_invalidate_accessnode.html is removed since there's nothing to test here any more because the root problem of bug 434464 (this test was introduced for) has gone
try server build (will be soon) https://build.mozilla.org/tryserver-builds/surkov.alexander@gmail.com-try-16ddad2671b8/.

Marco, it's necessary to test with JAWS.
Comment on attachment 450315 [details] [diff] [review]
patch

r=me; but I don't understand why we cached nsAccessNodes before, instead of accessible objects. Was it for performance for ISimpleDOM-only tools?

>-      accessible = do_QueryObject(GetCachedAccessNode(aNode));
>+      accessible = GetCachedAccessible(aNode);

Nice :)

>diff --git a/accessible/src/mac/nsAccessibleWrap.mm b/accessible/src/mac/nsAccessibleWrap.mm

>+PRBool
> nsAccessibleWrap::Init () 

>   if (!mNativeWrapper && !AncestorIsFlat()) {
>     // Create our native object using the class type specified in GetNativeType().
>     mNativeWrapper = new AccessibleWrapper (this, GetNativeType());
>   }

I noticed you added a bail here for nsDocAccessibleWrap::Init... why there and not here? When does it bite?
Attachment #450315 - Flags: review?(bolterbugz) → review+
(In reply to comment #3)
> (From update of attachment 450315 [details] [diff] [review])
> r=me; but I don't understand why we cached nsAccessNodes before, instead of
> accessible objects. Was it for performance for ISimpleDOM-only tools?

Is it general question? If so I don't know. If you ask how does the patch affects on them then nohow.

> >     mNativeWrapper = new AccessibleWrapper (this, GetNativeType());
> >   }
> 
> I noticed you added a bail here for nsDocAccessibleWrap::Init... why there and
> not here? When does it bite?

It should be here as well.
Comment on attachment 450315 [details] [diff] [review]
patch

This works with JAWS except the same problems as in bug 541618 current try-server build. But no netative effect on pages that do work like www.blindcooltech.com.
Attachment #450315 - Flags: review?(marco.zehe) → review+
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 450315 [details] [diff] [review] [details])
> > r=me; but I don't understand why we cached nsAccessNodes before, instead of
> > accessible objects. Was it for performance for ISimpleDOM-only tools?
> 
> Is it general question? If so I don't know. If you ask how does the patch
> affects on them then nohow.

General question.

> 
> > >     mNativeWrapper = new AccessibleWrapper (this, GetNativeType());
> > >   }
> > 
> > I noticed you added a bail here for nsDocAccessibleWrap::Init... why there and
> > not here? When does it bite?
> 
> It should be here as well.

OK thanks.
landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/6768f98d8cea
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Depends on: 571790
You need to log in before you can comment on or make changes to this bug.