The default bug view has changed. See this FAQ.

ASSERTION: There should always be a DOM node for an event: 'Not Reached', accessible/src/base/AccEvent.cpp, line 188

RESOLVED FIXED in mozilla10

Status

()

Core
Disability Access APIs
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

({access, regression})

unspecified
mozilla10
access, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 6 obsolete attachments)

(Assignee)

Description

8 years ago
Getting an assertion when opening new window

ASSERTION: There should always be a DOM node for an event: 'Not Reached', file c:/mozilla/fx07-22/accessible/src/base/nsAccessibleEventData.cpp, line 114

the problem we trying to fire reorder event for mParent when doc accessible is created but mParent is not initialized for root accessible. Either we should initialize mParent or use GetParent. Assertion appeared starting from bug 420845.
(Assignee)

Comment 1

8 years ago
Created attachment 390448 [details] [diff] [review]
patch

1) Make nsRootAccessible::GetParent to cache parent like it happens in nsDocAccessible
2) Call nsDocAcc::Init() after we append root acc to app root acc to fire reorder event when we constructed the tree
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #390448 - Flags: review?(marco.zehe)
Attachment #390448 - Flags: review?(ginn.chen)

Comment 2

8 years ago
Comment on attachment 390448 [details] [diff] [review]
patch

r=me with a nit:

>+      var expextedFirstChild = childCount > 0 ?
>+        children.queryElementAt(0, nsIAccessible) : null;

>+      is(firstChild, expextedFirstChild,
+         "Wrong first child of " + prettyName(acc));

These should be "expectedFirstChild", replace the second x with a c please.
Attachment #390448 - Flags: review?(marco.zehe) → review+
(Assignee)

Comment 3

8 years ago
Created attachment 390452 [details] [diff] [review]
patch
Attachment #390448 - Attachment is obsolete: true
Attachment #390452 - Flags: review?(ginn.chen)
Attachment #390448 - Flags: review?(ginn.chen)

Updated

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

Comment 4

8 years ago
landed on 1.9.2 http://hg.mozilla.org/mozilla-central/rev/5ead27dbc476
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

8 years ago
reopen since assertion is still presented because application root accessible hasn't DOM node.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

8 years ago
Created attachment 391793 [details] [diff] [review]
patch (next iteration)

hack fix to get rid assertion
(Assignee)

Updated

8 years ago
Attachment #391793 - Flags: review?(ginn.chen)

Comment 7

8 years ago
Comment on attachment 391793 [details] [diff] [review]
patch (next iteration)

r=me

"duiring" typo
Attachment #391793 - Flags: review?(ginn.chen) → review+
(Assignee)

Comment 8

8 years ago
landed on 1.9.2 - http://hg.mozilla.org/mozilla-central/rev/d6c10d88e4f6
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

8 years ago
not compiled on linux:

/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessibleEventData.cpp: In member function ‘void nsAccEvent::CaptureIsFromUserInput(PRBool)’:
/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessibleEventData.cpp:120: error: conversion from ‘nsRefPtr<nsApplicationAccessibleWrap>’ to non-scalar type ‘nsCOMPtr<nsIAccessible>’ requested
/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessibleEventData.cpp: In constructor ‘nsAccTextChangeEvent::nsAccTextChangeEvent(nsIAccessible*, PRInt32, PRUint32, PRBool, PRBool)’:
/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessibleEventData.cpp:568: warning: enumeral mismatch in conditional expression: ‘nsIAccessibleEvent::<anonymous enum>’ vs ‘nsIAccessibleEvent::<anonymous enum>’
../../../dist/include/nsAutoPtr.h: In destructor ‘nsRefPtr<T>::~nsRefPtr() [with T = nsApplicationAccessibleWrap]’:
/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessibleEventData.cpp:119:   instantiated from here
../../../dist/include/nsAutoPtr.h:956: error: invalid use of incomplete type ‘struct nsApplicationAccessibleWrap’
/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessNode.h:68: error: forward declaration of ‘struct nsApplicationAccessibleWrap’
../../../dist/include/nsAutoPtr.h: In copy constructor ‘nsRefPtr<T>::nsRefPtr(const nsRefPtr<T>&) [with T = nsApplicationAccessibleWrap]’:
/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessibleEventData.cpp:119:   instantiated from here
../../../dist/include/nsAutoPtr.h:972: error: invalid use of incomplete type ‘struct nsApplicationAccessibleWrap’
/builds/moz2_slave/mozilla-central-linux64/build/accessible/src/base/nsAccessNode.h:68: error: forward declaration of ‘struct nsApplicationAccessibleWrap’
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

8 years ago
I backed out this patch - http://hg.mozilla.org/mozilla-central/rev/86515f41793b
(Assignee)

Comment 11

8 years ago
Created attachment 392438 [details] [diff] [review]
patch2 (next iteration)
Attachment #391793 - Attachment is obsolete: true
Attachment #392438 - Flags: superreview?(neil)
(Assignee)

Comment 12

8 years ago
Created attachment 392440 [details] [diff] [review]
patch3 (next iteration)
Attachment #392438 - Attachment is obsolete: true
Attachment #392440 - Flags: superreview?(neil)
Attachment #392438 - Flags: superreview?(neil)
(Assignee)

Comment 13

8 years ago
Comment on attachment 392440 [details] [diff] [review]
patch3 (next iteration)

wrong patch again
Attachment #392440 - Attachment is obsolete: true
Attachment #392440 - Flags: superreview?(neil)
(Assignee)

Comment 14

8 years ago
Created attachment 392449 [details] [diff] [review]
patch4 (next iteration)
Attachment #392449 - Flags: superreview?(neil)
Attachment #392449 - Flags: review?(ginn.chen)

Updated

8 years ago
Attachment #392449 - Flags: review?(ginn.chen) → review+
Comment on attachment 392449 [details] [diff] [review]
patch4 (next iteration)

>+    // XXX: remove this hack during reorganization of 506907. Meanwhile we
>+    // want to get rid an assertion for application accessible events which
>+    // don't have DOM node (see bug 506206).
>+    nsRefPtr<nsApplicationAccessibleWrap> applicationAcc = 
>+      nsAccessNode::GetApplicationAccessible();
>+
>+    if (mAccessible != static_cast<nsIAccessible*>(applicationAcc.get()))
>+      NS_NOTREACHED("There should always be a DOM node for an event");
I still get this assertion even with the patch. In my case, mAccessible is null; this is presumably because a top-level chrome document has no parent.

Anyway this should all be in an #ifdef DEBUG block to avoid calling GetApplicationAccessible without using it.

Also if (!foo) NS_NOTREACHED("no foo"); should be NS_ASSERTION(foo, "no foo");
Comment on attachment 392449 [details] [diff] [review]
patch4 (next iteration)

>+    nsRefPtr<nsApplicationAccessibleWrap> applicationAcc = 
Trailing space ;-)
(Assignee)

Comment 17

8 years ago
(In reply to comment #15)

> >+    if (mAccessible != static_cast<nsIAccessible*>(applicationAcc.get()))
> >+      NS_NOTREACHED("There should always be a DOM node for an event");
> I still get this assertion even with the patch. In my case, mAccessible is
> null; this is presumably because a top-level chrome document has no parent.

I can get it when I investigate Error Console by DOMi, when I expand unaccessible iframe (because it is collapsed) which has accessible document (it shouldn't be accessible I think). I would like rather to deal with it in another bug since it's bit a different issue. If you like I can address this peculiarity in bug summary. Does it work for you?
(Assignee)

Comment 18

8 years ago
Created attachment 392654 [details] [diff] [review]
patch5 (next iteration)

with Neil's comments
Attachment #392449 - Attachment is obsolete: true
Attachment #392654 - Flags: superreview?(neil)
Attachment #392449 - Flags: superreview?(neil)
(Assignee)

Comment 19

8 years ago
Created attachment 392658 [details] [diff] [review]
patch6 (ni)

patch 5 is from another bug, sorry for the spam
Attachment #392654 - Attachment is obsolete: true
Attachment #392658 - Flags: superreview?(neil)
Attachment #392654 - Flags: superreview?(neil)
Well, this patch certainly improves things, although I still get the assertion whenever I open DOM Inspector more than once (the first time I guess it loads accessibility after the point at which the assertion would be triggered.)

>accessibility!nsAccEvent::CaptureIsFromUserInput(int aIsAsynch = 0)+0xc1 [mozilla\accessible\src\base\nsaccessibleeventdata.cpp @ 123]
>accessibility!nsAccEvent::nsAccEvent(unsigned int aEventType = 0x5c, class nsIAccessible * aAccessible = 0x00000000, int aIsAsynch = 0, nsAccEvent::EEventRule aEventRule = eCoalesceFromSameSubtree (1))+0x71 [mozilla\accessible\src\base\nsaccessibleeventdata.cpp @ 86]
>accessibility!nsAccReorderEvent::nsAccReorderEvent(class nsIAccessible * aAccTarget = 0x00000000, int aIsAsynch = 0, int aIsUnconditional = 1, class nsIDOMNode * aReasonNode = 0x0732aedc)+0x1b [mozilla\accessible\src\base\nsaccessibleeventdata.cpp @ 476]
>accessibility!nsDocAccessible::Init(void)+0x122 [mozilla\accessible\src\base\nsdocaccessible.cpp @ 626]
>accessibility!nsAccessibilityService::CreateRootAccessible(class nsIPresShell * aShell = 0x08a24220, class nsIDocument * aDocument = 0x0732ae40, class nsIAccessible ** aRootAcc = 0x0012f734)+0x651 [mozilla\accessible\src\base\nsaccessibilityservice.cpp @ 482]
>accessibility!nsAccessibilityService::GetAccessible(class nsIDOMNode * aNode = 0x0732aedc, class nsIPresShell * aPresShell = 0x08a24220, class nsIWeakReference * aWeakShell = 0x08a24dc8, class nsIFrame ** aFrameHint = 0x0012f770, int * aIsHidden = 0x0012f778, class nsIAccessible ** aAccessible = 0x0012f7f0)+0x2fa [mozilla\accessible\src\base\nsaccessibilityservice.cpp @ 1394]
>accessibility!nsAccessibilityService::GetAccessibleInShell(class nsIDOMNode * aNode = 0x0732aedc, class nsIPresShell * aPresShell = 0x08a24220, class nsIAccessible ** aAccessible = 0x0012f7f0)+0xf1 [mozilla\accessible\src\base\nsaccessibilityservice.cpp @ 1257]
>accessibility!nsAccessibilityService::GetAccessibleFor(class nsIDOMNode * aNode = 0x0732aedc, class nsIAccessible ** aAccessible = 0x0012f7f0)+0x12f [mozilla\accessible\src\base\nsaccessibilityservice.cpp @ 1201]
>accessibility!nsAccEvent::GetAccessibleByNode(void)+0xc4 [mozilla\accessible\src\base\nsaccessibleeventdata.cpp @ 288]
>accessibility!nsAccEvent::GetAccessible(class nsIAccessible ** aAccessible = 0x0012f958)+0x55 [mozilla\accessible\src\base\nsaccessibleeventdata.cpp @ 226]
>accessibility!nsDocAccessible::FlushPendingEvents(void)+0x16a [mozilla\accessible\src\base\nsdocaccessible.cpp @ 1651]
>accessibility!nsDocAccessible::FlushEventsCallback(class nsITimer * aTimer = 0x08289170, void * aClosure = 0x061765c8)+0x41 [mozilla\accessible\src\base\nsdocaccessible.cpp @ 1817]
>xpcom_core!nsTimerImpl::Fire(void)+0x28a [mozilla\xpcom\threads\nstimerimpl.cpp @ 427]
>xpcom_core!nsTimerEvent::Run(void)+0xa1 [mozilla\xpcom\threads\nstimerimpl.cpp @ 521]
>xpcom_core!nsThread::ProcessNextEvent(int mayWait = 1, int * result = 0x0012fa44)+0x1f3 [mozilla\xpcom\threads\nsthread.cpp @ 528]
>xpcom_core!NS_ProcessNextEvent_P(class nsIThread * thread = 0x00c9fb10, int mayWait = 1)+0x51 [c:\hg\mozilla\xpcom\build\nsthreadutils.cpp @ 230]
>gkwidget!nsBaseAppShell::Run(void)+0x5d [mozilla\widget\src\xpwidgets\nsbaseappshell.cpp @ 170]
>tkitcmps!nsAppStartup::Run(void)+0x69 [mozilla\toolkit\components\startup\src\nsappstartup.cpp @ 193]
>xul!XRE_main(int argc = 6, char ** argv = 0x00c9b870, struct nsXREAppData * aAppData = 0x00c9c640)+0x2a13 [mozilla\toolkit\xre\nsapprunner.cpp @ 3392]
>seamonkey!NS_internal_main(int argc = 6, char ** argv = 0x00c9b870)+0x111 [suite\app\nssuiteapp.cpp @ 103]
>seamonkey!wmain(int argc = 6, unsigned short ** argv = 0x00c99720)+0x101 [mozilla\toolkit\xre\nswindowswmain.cpp @ 108]
>seamonkey!wmainCRTStartup(void)+0x12c
kernel32!BaseProcessStart+0x23
Comment on attachment 392658 [details] [diff] [review]
patch6 (ni)

>+      NS_ASSERTION(eventNode, "There should always be a DOM node for an event");
Not exactly what I meant, but better than before at least.
Attachment #392658 - Flags: superreview?(neil) → superreview+
Weird. Even with the patch, I got an assertion ostensibly for a mouse move.
(Assignee)

Comment 23

8 years ago
part2 patch is landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/2826f35aa9ea, leaving the bug open.

Updated

8 years ago
Blocks: 444067
(Assignee)

Comment 24

6 years ago
(In reply to comment #22)
> Weird. Even with the patch, I got an assertion ostensibly for a mouse move.

Neil, I can't reproduce it on trunk. Can you?
(Assignee)

Updated

6 years ago
Summary: ASSERTION: There should always be a DOM node for an event: 'Not Reached', file c:/mozilla/fx07-22/accessible/src/base/nsAccessibleEventData.cpp, line 114 → ASSERTION: There should always be a DOM node for an event: 'Not Reached', file c:/mozilla/fx07-22/accessible/src/base/nsAccEvent.cpp, line 188
(Assignee)

Updated

6 years ago
Summary: ASSERTION: There should always be a DOM node for an event: 'Not Reached', file c:/mozilla/fx07-22/accessible/src/base/nsAccEvent.cpp, line 188 → ASSERTION: There should always be a DOM node for an event: 'Not Reached', file c:/mozilla/fx07-22/accessible/src/base/AccEvent.cpp, line 188
(Assignee)

Updated

6 years ago
Summary: ASSERTION: There should always be a DOM node for an event: 'Not Reached', file c:/mozilla/fx07-22/accessible/src/base/AccEvent.cpp, line 188 → ASSERTION: There should always be a DOM node for an event: 'Not Reached', accessible/src/base/AccEvent.cpp, line 188
(Assignee)

Comment 25

6 years ago
Created attachment 565493 [details] [diff] [review]
one more patch
Attachment #565493 - Flags: review?(bolterbugz)
Comment on attachment 565493 [details] [diff] [review]
one more patch

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

::: accessible/src/base/nsDocAccessible.cpp
@@ -1734,5 @@
>  {
> -  nsAccessible* accessible = aEvent->GetAccessible();
> -  if (!accessible)
> -    return;
> -

I don't see why this is okay to remove.
(Assignee)

Comment 27

6 years ago
(In reply to David Bolter [:davidb] from comment #26)

> ::: accessible/src/base/nsDocAccessible.cpp
> @@ -1734,5 @@
> >  {
> > -  nsAccessible* accessible = aEvent->GetAccessible();
> > -  if (!accessible)
> > -    return;
> > -
> 
> I don't see why this is okay to remove.

because of check in NotificationController
Comment on attachment 565493 [details] [diff] [review]
one more patch


>+    nsRefPtr<nsHyperTextAccessible> hyperText =
>+      do_QueryObject(aEvent->GetAccessible());

why not use AsHyperText()?

>+      nsRefPtr<AccEvent> caretMoveEvent = new AccCaretMoveEvent(hyperText,
>+                                                                caretOffset);
>       if (!caretMoveEvent)
>         return;

you don't need that null check.
(Assignee)

Comment 29

6 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #28)
> Comment on attachment 565493 [details] [diff] [review] [diff] [details] [review]
> one more patch

fixed after our irc chat :) thanks for watching!
Comment on attachment 565493 [details] [diff] [review]
one more patch

>+      ShutdownChildrenInSubtree(aEvent->GetAccessible());

I think we should null check, probably in ShutdownChildrenInSubtree.

r=me with that and Trevor's comments addressed.
Attachment #565493 - Flags: review?(bolterbugz) → review+
(Assignee)

Comment 31

6 years ago
(In reply to David Bolter [:davidb] from comment #30)
> Comment on attachment 565493 [details] [diff] [review] [diff] [details] [review]
> one more patch
> 
> >+      ShutdownChildrenInSubtree(aEvent->GetAccessible());
> 
> I think we should null check, probably in ShutdownChildrenInSubtree.
> 
> r=me with that and Trevor's comments addressed.

null check aEvent->GetAccessible()? Why?
(In reply to alexander surkov from comment #31)
> (In reply to David Bolter [:davidb] from comment #30)

null check aEvent->GetAccessible()? Why?

OK I see, we're ok.
(Assignee)

Comment 33

6 years ago
inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/bb3764df1a70
https://hg.mozilla.org/mozilla-central/rev/bb3764df1a70
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.