666337 - "ASSERTION: null data pointer" in nsAccessibleWrap::FirePlatformEvent with <audio>

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Jesse Ruderman]

Attachment: testcase

1. Enable accessibility, e.g. by pasting the following into the js console:

Components.classes["@mozilla.org/accessibilityService;1"]
      .getService(Components.interfaces.nsIAccessibleRetrieval);

2. Load the testcase.

3. Wait for the audio to finish? Reload the page?

Result:

###!!! ASSERTION: null data pointer: 'Not Reached', file /builds/slave/cen-lnx64-dbg/build/xpcom/string/src/nsTSubstring.cpp, line 615

nsACString_internal::Equals [xpcom/string/src/nsTSubstring.cpp:616]
nsAccessibleWrap::FirePlatformEvent [accessible/src/atk/nsAccessibleWrap.cpp:1085]
nsAccessibleWrap::HandleAccEvent [accessible/src/atk/nsAccessibleWrap.cpp:1033]
nsEventShell::FireEvent [accessible/src/base/nsEventShell.cpp:65]
nsDocAccessible::ProcessPendingEvent [accessible/src/base/nsDocAccessible.cpp:1720]
NotificationController::WillRefresh [accessible/src/base/NotificationController.cpp:306]
nsRefreshDriver::Notify [layout/base/nsRefreshDriver.cpp:328]
nsTimerImpl::Fire [xpcom/threads/nsTimerImpl.cpp:428]
nsTimerEvent::Run [xpcom/threads/nsTimerImpl.cpp:522]
nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:618]
NS_ProcessNextEvent_P [obj-firefox/xpcom/build/nsThreadUtils.cpp:245]
mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:107]
MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:219]
MessageLoop::RunHandler [ipc/chromium/src/base/message_loop.cc:203]
MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:175]
nsBaseAppShell::Run [widget/src/xpwidgets/nsBaseAppShell.cpp:191]
nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:222]
XRE_main [toolkit/xre/nsAppRunner.cpp:3565]
do_main [browser/app/nsBrowserApp.cpp:198]
main [browser/app/nsBrowserApp.cpp:281]
libc.so.6 + 0x1eeff
firefox-bin + 0x1139

Regression from bug 659018?

Comment 1

[Trevor Saunders (:tbsaunde)]

(In reply to comment #0)
> Created attachment 541130 [details]
> testcase
> 
> 1. Enable accessibility, e.g. by pasting the following into the js console:
> 
> Components.classes["@mozilla.org/accessibilityService;1"]
>       .getService(Components.interfaces.nsIAccessibleRetrieval);

yeah, we know you can save yourself the time :)

> ###!!! ASSERTION: null data pointer: 'Not Reached', file
> /builds/slave/cen-lnx64-dbg/build/xpcom/string/src/nsTSubstring.cpp, line 615
> 
> nsACString_internal::Equals [xpcom/string/src/nsTSubstring.cpp:616]
> nsAccessibleWrap::FirePlatformEvent
> [accessible/src/atk/nsAccessibleWrap.cpp:1085]
> nsAccessibleWrap::HandleAccEvent
> [accessible/src/atk/nsAccessibleWrap.cpp:1033]

ok, so atkobj->name can reasonably be null.  So the fix is trivial just atkObj->name && utf8Name.Equals(atkObj->name).  However   I don't understand why we assert that the arg to nsTSubstring::Equal() isn't Null.  dbaron asked for that assertion in bug  236003.  Off hand I don't understand why comparing to a null string is unreasonable, but I'm willing to accept that it is given a small amount of reason :-)

> Regression from bug 659018?

well yes, in some sense of the word regression ;)

Comment 2

[alexander :surkov (:asurkov)]

trevor, trivial patch?

Comment 3

[Trevor Saunders (:tbsaunde)]

(In reply to comment #2)
> trevor, trivial patch?

yes, I basically mentioned what it was in my comment, but see the other part too.

> atkObj->name && utf8Name.Equals(atkObj->name).  However   I don't understand
> why we assert that the arg to nsTSubstring::Equal() isn't Null.  dbaron
> asked for that assertion in bug  236003.  Off hand I don't understand why
> comparing to a null string is unreasonable, but I'm willing to accept that
> it is given a small amount of reason :-)

Comment 4

[alexander :surkov (:asurkov)]

maybe because they want to deal with stings making null pointer out of law. Anyway it's xpcom guys decision, until we have real objections we should follow it.

Comment 5

[Trevor Saunders (:tbsaunde)]

Attachment: add null check

this makes me a bit sad, but I gues if we need to fix this in code we control we need this patch.

Comment 6

[Trevor Saunders (:tbsaunde)]

landed http://hg.mozilla.org/mozilla-central/rev/e5a39bc7d5d6