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

RESOLVED FIXED in mozilla7

Status

()

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

People

(Reporter: Jesse Ruderman, Assigned: tbsaunde)

Tracking

({assertion, regression, testcase})

1.9.2 Branch
mozilla7
x86_64
Linux
assertion, regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
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);

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?
(Assignee)

Comment 1

6 years ago
(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

6 years ago
trevor, trivial patch?
Assignee: nobody → trev.saunders
(Assignee)

Comment 3

6 years ago
(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

6 years ago
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.
(Assignee)

Comment 5

6 years ago
Created attachment 541893 [details] [diff] [review]
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.
Attachment #541893 - Flags: review?(surkov.alexander)

Updated

6 years ago
Attachment #541893 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 6

6 years ago
landed http://hg.mozilla.org/mozilla-central/rev/e5a39bc7d5d6
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.