Bug 138727 - AX: [ATK] Crash getting the orientation of a MenuListOption after the MenuList was removed from the document
Summary: AX: [ATK] Crash getting the orientation of a MenuListOption after the MenuLis...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joanmarie Diggs
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-11-13 21:37 PST by Joanmarie Diggs
Modified: 2014-11-18 04:51 PST (History)
11 users (show)

See Also:


Attachments
Patch (5.86 KB, patch)
2014-11-13 21:49 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Patch (5.75 KB, patch)
2014-11-14 03:46 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 (586.00 KB, application/zip)
2014-11-14 04:44 PST, Build Bot
no flags Details
Patch (5.76 KB, patch)
2014-11-14 06:06 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs 2014-11-13 21:37:35 PST
AccessibilityMenuListOption::elementRect() returns the value of the grandparent MenuList. If we have an existing MenuListOption and the widget backing MenuList was just removed from the document, calling AccessibilityMenuListOption::elementRect() will crash the web process.
Comment 1 Radar WebKit Bug Importer 2014-11-13 21:37:58 PST
<rdar://problem/18980391>
Comment 2 Joanmarie Diggs 2014-11-13 21:49:53 PST
Created attachment 241555 [details]
Patch
Comment 3 Mario Sanchez Prada 2014-11-14 02:53:20 PST
Comment on attachment 241555 [details]
Patch

Sounds reasonable to me, but you could just early return if the null-check fails before the ASSERT, and leave the assertion in place. That way, if the parent/grandparent is not null, you would still be asserting the right role.
Comment 4 Joanmarie Diggs 2014-11-14 03:46:35 PST
Created attachment 241574 [details]
Patch
Comment 5 Build Bot 2014-11-14 04:44:16 PST
Comment on attachment 241574 [details]
Patch

Attachment 241574 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6306052337303552

Number of test failures exceeded the failure limit.
Comment 6 Build Bot 2014-11-14 04:44:20 PST
Created attachment 241581 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 7 Joanmarie Diggs 2014-11-14 06:06:24 PST
Created attachment 241584 [details]
Patch
Comment 8 Joanmarie Diggs 2014-11-14 07:08:21 PST
Comment on attachment 241584 [details]
Patch

The previous, failureful version returned LayoutRect(). This version returns boundingBoxRect() which is what often gets returned for non MenuList elements. According to the EWS and my Mac Mini, returning boundingBoxRect() here seems sane.

Having said that, I was surprised at how a sanity check in accessibility code could break so many non-accessibility tests on the Mac. So... Chris, please review. Thanks!!
Comment 9 WebKit Commit Bot 2014-11-18 04:51:52 PST
Comment on attachment 241584 [details]
Patch

Clearing flags on attachment: 241584

Committed r176254: <http://trac.webkit.org/changeset/176254>
Comment 10 WebKit Commit Bot 2014-11-18 04:51:57 PST
All reviewed patches have been landed.  Closing bug.