Closed Bug 377470 Opened 17 years ago Closed 17 years ago

Crash [@ nsAccessible::GetState] using accessibility api and moving stuff

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: martijn.martijn, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: crash, regression, testcase, Whiteboard: [sg:critical?] destroyed frame, post 1.8-branch)

Crash Data

Attachments

(3 files, 1 obsolete file)

Attached file testcase
I'm marking this security sensitive, because this uses Mats' script from bug 376924 (thanks for that, Mats).
If it doesn't need to be security sensitive, please open the bug.

The testcase doesn't seem to crash on branch, so it seems like a regression.

Talkback ID: TB31175040E
nsAccessible::GetState  [mozilla/accessible/src/base/nsaccessible.cpp, line 1018]
nsHTMLComboboxListAccessible::GetState  [mozilla/accessible/src/html/nshtmlselectaccessible.cpp, line 1303]
nsAccessible::GetFinalState  [mozilla/accessible/src/base/nsaccessible.cpp, line 2329]
nsAccessible::State  [mozilla/accessible/src/base/nsaccessible.h, line 192]
nsAccessible::GetMultiSelectFor  [mozilla/accessible/src/base/nsaccessible.cpp, line 1346]
nsHTMLSelectOptionAccessible::SelectionChangedIfOption  [mozilla/accessible/src/html/nshtmlselectaccessible.cpp, line 761]
nsDocAccessible::ContentStatesChanged  [mozilla/accessible/src/base/nsdocaccessible.cpp, line 1039]
nsHTMLOptionElement::SetSelectedInternal  [mozilla/content/html/content/src/nshtmloptionelement.cpp, line 226]
nsHTMLSelectElement::OnOptionSelected  [mozilla/content/html/content/src/nshtmlselectelement.cpp, line 1226]
nsHTMLSelectElement::SetOptionsSelectedByIndex  [mozilla/content/html/content/src/nshtmlselectelement.cpp, line 1379]
nsHTMLSelectElement::SetSelectedIndex  [mozilla/content/html/content/src/nshtmlselectelement.cpp, line 1175]
nsHTMLSelectElement::RemoveOptionsFromList  [mozilla/content/html/content/src/nshtmlselectelement.cpp, line 732]
nsHTMLSelectElement::WillRemoveOptions  [mozilla/content/html/content/src/nshtmlselectelement.cpp, line 899]
nsHTMLSelectElement::RemoveChildAt  [mozilla/content/html/content/src/nshtmlselectelement.cpp, line 586]
nsGenericElement::InsertBefore  [mozilla/content/base/src/nsgenericelement.cpp, line 2537]
nsGenericElement::AppendChild  [mozilla/content/base/src/nsgenericelement.h, line 530]
XPCWrappedNative::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2247]
The testcase uses enhanced privileges, so you need to download it to your computer to see the crash.
Attached file stack
Assignee: aaronleventhal → mats.palmgren
OS: Windows XP → All
Hardware: PC → All
Attached patch Patch rev. 1 (obsolete) — Splinter Review
Attachment #261543 - Flags: review?(aaronleventhal)
Whiteboard: [sg:critical?] destroyed frame
Comment on attachment 261543 [details] [diff] [review]
Patch rev. 1

I haven't tested what this does to our implementation,  and whether we still return the right frame. Does it?

The constructor shouldn't  take aListFrame any more should it? What is it used for now?
Attachment #261543 - Flags: review?(aaronleventhal) → review-
Is the crash really because of bug 357583, and we're not getting notifications from layout because a11y is initialized by JS?
Attached patch Patch rev. 2Splinter Review
Now returning the right frame...
Attachment #261543 - Attachment is obsolete: true
Attachment #261589 - Flags: review?(aaronleventhal)
Comment on attachment 261589 [details] [diff] [review]
Patch rev. 2

Thanks for the fix!

Just a nit, s/f/frame. I prefer multiple letter variable names, because it makes it easier to search for them in the code, and are slightly more readable IMO.

Also, the code does slightly less work if you do:

+  if (frame) {
+    nsIComboboxControlFrame* comboBox;
+    CallQueryInterface(f, &comboBox);
+    if (comboBox) {
+      return comboBox->GetDropDown();
+    }
+  }
+  return nsnull;
}
Attachment #261589 - Flags: review?(aaronleventhal) → review+
I checked it in with that change. Thanks for the fix Mats.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
(In reply to comment #0)
> The testcase doesn't seem to crash on branch, so it seems like a regression.

Crashed for me in a debug FF2.0.0.4pre build. kinda hung, then crashed when I started typing in the input field. Stack is different so could be something else (in which case we should create a branch-only bug). mFirstChild in nsAccessible::Shutdown points at an object with a deleted vtbl (0xdddddddd).

>	nsCOMPtr<nsIAccessible>::nsCOMPtr<nsIAccessible>() Line 627
 	nsAccessible::Shutdown() Line 455
 	nsAccessNode::ClearCacheEntry() Line 530
 	nsBaseHashtable<nsVoidHashKey,nsCOMPtr<nsIAccessNode>,nsIAccessNode *>::s_EnumStub() Line 346
 	PL_DHashTableEnumerate() Line 683
 	nsBaseHashtable<nsVoidHashKey,nsCOMPtr<nsIAccessNode>,nsIAccessNode *>::Enumerate() Line 221
 	nsAccessNode::ClearCache() Line 538
 	nsDocAccessible::Shutdown() Line 483
 	nsDocAccessibleWrap::Shutdown() Line 173
 	nsAccessNode::ClearCacheEntry() Line 530
 	nsBaseHashtable<nsVoidHashKey,nsCOMPtr<nsIAccessNode>,nsIAccessNode *>::s_EnumStub() Line 346
 	PL_DHashTableEnumerate() Line 683
 	nsBaseHashtable<nsVoidHashKey,nsCOMPtr<nsIAccessNode>,nsIAccessNode *>::Enumerate() Line 221
 	nsAccessNode::ClearCache() Line 538
 	nsAccessNode::ShutdownXPAccessibility() Line 214
 	nsAccessNodeWrap::ShutdownAccessibility() Line 588
 	nsAccessibilityService::Observe() Line 154
 	nsObserverService::NotifyObservers() Line 233
 	NS_ShutdownXPCOM_P() Line 812
 	ScopedXPCOMStartup::~ScopedXPCOMStartup() Line 742
 	XRE_main() Line 2743
 	main() Line 61
 	mainCRTStartup() Line 398
Flags: wanted1.8.1.x?
Flags: wanted1.8.0.x?
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x?
Flags: wanted1.8.0.x+
Blocks: 386382
Created bug 386382 for the branch shutdown crash described in comment 9
This particular patch doesn't apply to the 1.8 branch; it tweaks some of the changes that were landed in bug 278034 which was post-1.8 only. Whatever is going on in bug 386382 is completely unrelated to what this bug is fixing, despite being discovered by the same testcase.
Blocks: 278034
Flags: wanted1.8.1.x-
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x-
Flags: wanted1.8.0.x+
Whiteboard: [sg:critical?] destroyed frame → [sg:critical?] destroyed frame, post 1.8-branch
Group: security
Flags: in-testsuite?
Crash Signature: [@ nsAccessible::GetState]
You need to log in before you can comment on or make changes to this bug.