393141 - Crash [@ nsAccessibilityService::GetAccessible] with display:none option inside optgroup

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, because of the used of enhanced privileges, you need to download the file to your desktop.

It also crashes branch builds, so marking security sensitive for now.

http://crash-stats.mozilla.com/report/index/df3d0fd9-5063-11dc-968c-001a4bd43ed6
0  	xul.dll@0x59c62d  	
1 	nsAccessibilityService::GetAccessible(nsIDOMNode*, nsIPresShell*, nsIWeakReference*, nsIFrame**, int*, nsIAccessible**) 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\accessible\src\base\nsAccessibilityService.cpp:1209
2 	nsAccessibilityService::GetAccessibleInWeakShell(nsIDOMNode*, nsIWeakReference*, nsIAccessible**) 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\accessible\src\base\nsAccessibilityService.cpp:1143
3 	nsHTMLSelectListAccessible::AccessibleForOption(nsIAccessibilityService*, nsIContent*, nsIAccessible*, int*) 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\accessible\src\html\nsHTMLSelectAccessible.cpp:367
4 	nsHTMLSelectListAccessible::CacheOptSiblings(nsIAccessibilityService*, nsIContent*, nsIAccessible*, int*) 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\accessible\src\html\nsHTMLSelectAccessible.cpp:408
5 	nsHTMLSelectListAccessible::CacheChildren() 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\accessible\src\html\nsHTMLSelectAccessible.cpp:445
6 	nsAccessible::GetChildCount(int*) 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\accessible\src\base\nsAccessible.cpp:798
7 	nsAccessible::GetFirstChild(nsIAccessible**) 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\accessible\src\base\nsAccessible.cpp:682
8 	NS_InvokeByIndex_P 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp:101
9 	AutoJSSuspendRequest::SuspendRequest() 	e:\builds\tinderbox\fx-trunk-newref\winnt_5.2_depend\mozilla\js\src\xpconnect\src\xpcprivate.h:3313
10 	RtlFreeHeap

Comment 1

[Aaron Leventhal]

Attachment: Don't set lastGoodAccessible if AccessibleForOption() fails or CacheOptSiblings() returns null, otherwise we can't exit the method properly

Comment 2

[Martijn Wargers (dead)]

Aaron, you forgot to set the mail address to ask review.

Comment 3

[Evan Yan]

Aaron, could you drop a line about and how it crashed and how the patch fixed the crash?

Comment 4

[Aaron Leventhal]

Evan, I don't know exactly how it crashed except that the cache was messed up. I had a hunch that the problem was in the special code that caches option and optgroup. I looked in the code and saw a mistake in it right away. Once I fixed that mistake, the crash went away.

Since it is clearly the correct thing to do and fixes the issue I felt it was not necessary to spend more time on it.

Comment 5

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007091904 Minefield/3.0a8pre

Comment 6

[Daniel Veditz [:dveditz]]

The 1.8 branch crashes on a deleted mFirstChild in  nsAccessibilityService::GetAccessible.

Comment 7

[Daniel Veditz [:dveditz]]

Does this patch work for the 1.8 branch or do we need another? Please request approval on the appropriate patch

Comment 8

[Samuel Sidler (old account; do not CC)]

For reference: tb39990981

Comment 9

[Daniel Veditz [:dveditz]]

Attachment: 1.8 branch version

straightforward merge, IsContentOfType() instead of IsNodeOfType() and no aChildCount on two function calls.

Comment 10

[Samuel Sidler (old account; do not CC)]

Comment on attachment 299679 [details] [diff] [review]
1.8 branch version

Approved to land on the branch for 1.8.1.12. a=ss

Comment 11

[Daniel Veditz [:dveditz]]

patch checked in on 1.8 branch

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.12pre) Gecko/2008012803 BonEcho/2.0.0.12pre. Crashes in 2.0.0.11 but no crash with nightly.

Comment 13

[Alexander Sack]

Comment on attachment 299679 [details] [diff] [review]
1.8 branch version

a=asac for 1.8.0.15

(unmodified distro patch).

Comment 14

[Reed Loden [:reed]]

MOZILLA_1_8_0_BRANCH:

Checking in accessible/src/html/nsHTMLSelectAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLSelectAccessible.cpp,v  <--  nsHTMLSelectAccessible.cpp
new revision: 1.46.2.2.2.3; previous revision: 1.46.2.2.2.2
done