The testcase uses enhanced privileges, so you need to download it to your computer to get the crash. Usually, it crashes for me after a few reloads. It doesn't seem to crash on branch. Talkback ID: TB31278004H table Talkback ID: TB31277659M 0x02706448 nsQueryInterface::operator() [mozilla/xpcom/build/nscomptr.cpp, line 52] nsCOMPtr<nsISupports>::operator= [mozilla/dist/include/xpcom/nscomptr.h, line 1056] XPCConvert::NativeInterface2JSObject [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 1104] XPCConvert::NativeData2JS [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 480] XPCWrappedNative::CallMethod [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2337]

I don't think we should file any more bugs on the Javascript-only testcases for accessibility until we fix bug 357583.

So I guess this depends on a fix for bug 357583 then?

Okay, bug 357583 is checked in -- let's get back to these JS testcases! :)

I'm still getting this crash with a 2007-05-06 build, which is after bug 357583 was fixed.

Aaron changed something in bug 268935 that makes the original testcase not work anymore with current trunk builds. This is the updated testcase, using nsIAccessibleRetrieval. Btw, this type of crash I often get in my testing. But I'm not sure if it's related to this one, or something different.

FWIW, I can't reproduce this on Linux.

Aaron, this looks exploitable based on the stack in comment 0, so I hope you'll be able to fix it soon :)

Yikes, this testcase is difficult to understand. It refers to a textarea element that is not even there.

Maybe the testcase from bug 381010 is easier to understand?

If I completely remove nsHTMLTableAccessible::CacheChildren() the bug goes away. Martijn, can you verify that?

Sorry, I don't have a build with accessibility enabled (mingw), so I can't verify.

Well, I wonder if this broke on 3/28 when the fix for bug 360184 went in.

I don't crash with a 2007-03-25 build, but I do crash with a 2007-03-27 build. Cause by bug 360184 (but that went into the branch) or bug 371594?

I don't think it's bug 371594.

I think it has to do with our code to try and deal with captions in a table. It assumes the caption accessible is a direct child of the table, but actually it can be anywhere. That could happen on Windows more where we create an accessible object for thead, tbody and tfoot. Comment 6 verifies that this bug doesn't occur on Linux. It's a theory, anyway :)

The inclusion of tbody/thead/tfoot accessibles doesn't affect this as I thought, so I'm not sure why the bug doesn't occur on Linux for Mats.

Comment on attachment 270325 [details] [diff] [review] Same affect is fix for bug 360184 but simpler and doesn't cause crash when a html table has multiple <caption>, this patch doesn't work, because nsAccessible::CacheChildren() will create multiple captionAccessible.

Evan, what needs to happen when there are multiple captions? Do we need to remove the extras or use them for something? Do you want to take the bug? It's happening in your code somehow.

Maybe bug 243159, comment 20 is useful? It's talking about multiple table captions in layout code.

(In reply to comment #19) > Evan, what needs to happen when there are multiple captions? Do we need to > remove the extras or use them for something? > I think removing the extras is the right thing to do. However we can't get the extra captions from GetCaption(), that's why I added the condition in traversing DOM tree. > Do you want to take the bug? It's happening in your code somehow. > I'd like to but I don't have a windows environment. Which part of the former code caused the crash? I think it doesn't assume the caption accessible need to be a direct child of the table.

I don't know which part caused the crash to be honest, but when I removed or reimplemented the code it fixed the bug.

does this help?

Comment on attachment 270585 [details] [diff] [review] just a try, to move caption handling code after No, the bug still occurs.

Evan, sorry for the large patch. This is based on my earlier fix but I found more things to clean up, mostly related to the use of captions, after your comment about multiple captions. There was also the removal of SetCaption/SetSummary (see item #4). 1) Base the creation of caption accessibles on the HTML frame (created via display: table-caption) 2) Add nsHTMLCaptionAccessible class, which supports description_for relation (it describes the table) 3) Do not create the accessibles for non-visible captions 4) Remove the ability to set caption or summary from nsIAccessibleTable -- the API should not mutate the HTML document! 5) Reuse nsAccessible::CacheChildren() and move caption accessible to first child 6) Simplify GetCaption() to just use first child and check if it's a caption 7) Support description relation 8) Support summary=name, caption=description

Comment on attachment 270901 [details] [diff] [review] Deal with multiple captions and other cleanup, see comment firefox crash at start, core stack: #0 0x00002b5b34d3e1d1 in nanosleep () from /lib/libc.so.6 #1 0x00002b5b34d3dff4 in sleep () from /lib/libc.so.6 #2 0x00002b5b2edaea67 in ah_crap_handler (signum=11) at nsSigHandlers.cpp:135 #3 0x00002b5b2edc3aed in nsProfileLock::FatalSignalHandler (signo=11) at nsProfileLock.cpp:210 #4 <signal handler called> #5 0x00002b5b33ddebbc in g_type_check_instance_cast () from /usr/lib/libgobject-2.0.so.0 #6 0x00002b5b2fc1e0bf in nsAccessibleWrap::GetAtkObject (acc=0x2aaab00ae310) at ../../../../mozilla/accessible/src/atk/nsAccessibleWrap.cpp:407 #7 0x00002b5b2fc24861 in nsApplicationAccessibleWrap::RemoveRootAccessible (this=0x11b70f0, aRootAccWrap=0x2aaab00ae310) at ../../../../mozilla/accessible/src/atk/nsAppRootAccessible.cpp:627 #8 0x00002b5b2fbdf496 in nsAccessibilityService::RemoveNativeRootAccessible (this=0x11db310, aRootAccessible=0x2aaab00ae310) at ../../../../mozilla/accessible/src/base/nsAccessibilityService.cpp:1752 #9 0x00002b5b2f08a589 in nsFrameManager::ReResolveStyleContext (this=0x2aaab00ae348, aPresContext=0x2aaab00a00c0, aFrame=0x16d8160, aParentContent=0x16017e0, aChangeList=0x7fff7c5831e0, aMinChange=7) at ../../../mozilla/layout/base/nsFrameManager.cpp:1386 >Index: accessible/public/nsIAccessibleTable.idl >=================================================================== >RCS file: /cvsroot/mozilla/accessible/public/nsIAccessibleTable.idl,v >- attribute nsIAccessible caption; >- attribute AString summary; >+ readonly attribute nsIAccessible caption; >+ readonly attribute AString summary; these change also affects xul tree table, please modify them also.

The startup crash is because your vtbl for a11y service is now out of whack. You need to rebuild anywhere that uses a11yservice.

Comment on attachment 271042 [details] [diff] [review] Remove summary/caption support for XUL trees r=me

Comment on attachment 271042 [details] [diff] [review] Remove summary/caption support for XUL trees Technically we need a layout peer to review the small changes to layout, even though they're #ifdef ACCESSIBILITY

Comment on attachment 271042 [details] [diff] [review] Remove summary/caption support for XUL trees >Index: layout/tables/nsTableOuterFrame.cpp Please remove the #include "nsIAccessibilityService.h" as well. r+sr=mats for the layout changes

We still need that #include there, we're adding an addition user of the service in nsTableOuterFrame. Or am I misunderstanding you?

Nevermind my previous comment, I made a mistake, the patch looks fine as is.

Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007070604 Minefield/3.0a7pre Thanks for fixing!