884251 - "ASSERTION: Queued event belongs to another document"

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Jesse Ruderman]

Attachment: testcase

With accessibility enabled, the testcase hits:

###!!! ASSERTION: Queued event belongs to another document!: '(aEvent->mAccessible && aEvent->mAccessible->IsApplication()) || aEvent->GetDocAccessible() == mDocument', file accessible/src/base/EventQueue.cpp, line 28

Comment 1

[Jesse Ruderman]

Attachment: stack (gdb)

Comment 2

[Jesse Ruderman]

Now it also triggers:

###!!! ASSERTION: Unbinding the unbound accessible!: 'mAccessibleCache.GetWeak(aAccessible->UniqueID())', file accessible/src/generic/DocAccessible.cpp, line 1308

Comment 3

[Trevor Saunders (:tbsaunde)]

this is actually pretty simple.  When the href attribute changes we call RecreateAccessible(iframeNode) which then calls UpdateTree(outerDocForIframe, htmlLinkNode, false) then the outerDocAccessible has one child the document for the iframe so we try and handle the doc accessible as a standard accessible child in the loop at DocAccessible.cpp:1802 So UpdateTreeInternal() will try to fire a hide event on the doc accessible.  That event's target is the iframe document and the document its fired in is the parent document. So the assertion aEvent->mAccessible->Document() == mDocument is false.  I'm tempted to just skip doc accessible children in that loop seem reasonable?

Comment 4

[Trevor Saunders (:tbsaunde)]

Attachment: bug 884251 - DocAccessible::UpdateTree() doesn't need to handle updating children that are sub documents

Comment 5

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #3)
> this is actually pretty simple.  When the href attribute changes we call
> RecreateAccessible(iframeNode) which then calls
> UpdateTree(outerDocForIframe, htmlLinkNode, false) then the
> outerDocAccessible has one child the document for the iframe so we try and
> handle the doc accessible as a standard accessible child in the loop at
> DocAccessible.cpp:1802 So UpdateTreeInternal() will try to fire a hide event
> on the doc accessible. 

that link element is not accessible, right? and has no any connection with underlying iframe document? So why do we touch that document accessible?

Comment 6

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #5)
> (In reply to Trevor Saunders (:tbsaunde) from comment #3)
> > this is actually pretty simple.  When the href attribute changes we call
> > RecreateAccessible(iframeNode) which then calls
> > UpdateTree(outerDocForIframe, htmlLinkNode, false) then the
> > outerDocAccessible has one child the document for the iframe so we try and
> > handle the doc accessible as a standard accessible child in the loop at
> > DocAccessible.cpp:1802 So UpdateTreeInternal() will try to fire a hide event
> > on the doc accessible. 
> 
> that link element is not accessible, right? and has no any connection with
> underlying iframe document? So why do we touch that document accessible?

iirc the container node its being removed from is the <iframe>. So we call GetAccessibleOrContainer(iframeNode) which gets us the OuterDoc accessible.  Then UpdateTree() iterates over the kids which is [ Doc ]

Comment 7

[alexander :surkov (:asurkov)]

rephrasing the question, you said that @href removal on child link of iframe results in RecreateAccessible(iframeNode), why does it happen?

Comment 8

[alexander :surkov (:asurkov)]

Comment on attachment 8348818 [details] [diff] [review]
bug 884251 - DocAccessible::UpdateTree() doesn't need to handle updating children that are sub documents

Review of attachment 8348818 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/generic/DocAccessible.cpp
@@ +1804,5 @@
>  
>          // If accessible doesn't have its own content then we assume parent
> +        // will handle its update.  If child is DocAccessible then we don't
> +        // handle updating it here either.
> +        if (!child->HasOwnContent() || child->IsDoc()) {

a comment for the record: if we don't skip document then we fall into loop below, we can't cross document boundaries so chilNode is null and thus document accessible gets removed from the tree

the bug looks like a very important bug

also it'd be good to have a mochitest, that makes sure after @href on link was changed there's a document accessible still in the tree. Trev, does it sound good?

Comment 9

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #8)
> Comment on attachment 8348818 [details] [diff] [review]
> bug 884251 - DocAccessible::UpdateTree() doesn't need to handle updating
> children that are sub documents
> 
> Review of attachment 8348818 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/generic/DocAccessible.cpp
> @@ +1804,5 @@
> >  
> >          // If accessible doesn't have its own content then we assume parent
> > +        // will handle its update.  If child is DocAccessible then we don't
> > +        // handle updating it here either.
> > +        if (!child->HasOwnContent() || child->IsDoc()) {
> 
> a comment for the record: if we don't skip document then we fall into loop
> below, we can't cross document boundaries so chilNode is null and thus
> document accessible gets removed from the tree

am I supposed to do something with this or are you just noting it?

> the bug looks like a very important bug

not quiet sure why you think that, but I guess it doesn't really matter.

> also it'd be good to have a mochitest, that makes sure after @href on link
> was changed there's a document accessible still in the tree. Trev, does it
> sound good?

can you live with another better tests follow up bug pls?

Comment 10

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #9)

> > a comment for the record: if we don't skip document then we fall into loop
> > below, we can't cross document boundaries so chilNode is null and thus
> > document accessible gets removed from the tree
> 
> am I supposed to do something with this or are you just noting it?

no, just a notion if somebody will have questions later

> > the bug looks like a very important bug
> 
> not quiet sure why you think that, but I guess it doesn't really matter.

say if you have a document like

<iframe src="www.mozilla.org"><link></link></iframe>

then you set @href attribute on link then would AT have an access to mozilla.org site? From what I can see we will remove the document accessible and shutdown it and nothing seems will trigger it recreation

> > also it'd be good to have a mochitest, that makes sure after @href on link
> > was changed there's a document accessible still in the tree. Trev, does it
> > sound good?
> 
> can you live with another better tests follow up bug pls?

sure

Comment 11

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1b7722aa79d

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b1b7722aa79d