Bug 75279

Summary: Crash in the WebKit accessibility code while attempting to retrieve the title UI element.
Product: WebKit Reporter: Ananta Iyengar <ananta>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch
none
proposed patch with description updated
none
Patch with review comments addressed none

Description Ananta Iyengar 2011-12-27 16:43:21 PST
We have been seeing this crash in Chromium with accessibility enabled. The chromium bug is here
http://crbug.com/108508(Logged against Chromeframe tests which enable webkit accessibility).

Debugging revealed that the crash occurs in the AccessibilityRenderObject::titleUIElement method
because of a NULL node being returned by the underlying RenderObject. Debugging this function
revealed that the RenderObject can return a NULL node pointer at times(if it is anonymous). 

We should check for a NULL node here.
Will upload a patch in a bit
Comment 1 Ananta Iyengar 2011-12-27 19:55:28 PST
Created attachment 120629 [details]
proposed patch
Comment 2 Ryosuke Niwa 2011-12-27 20:02:05 PST
Comment on attachment 120629 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120629&action=review

> Source/WebCore/ChangeLog:3
> +        https://bugs.webkit.org/show_bug.cgi?id=75279

You need to have a bug summary "Crash in the WebKit accessibility code while attempting to retrieve the title UI element." directly above the bug url. See other change log entries.

> Source/WebCore/ChangeLog:5
> +        Fix a crash in the the WebKit accessibility code which occurs while retrieving
> +        the title UI clement. The fix is to NULL check the RenderObject::node return value.

This line should appear below "Reviewed by" followed by a blank line. See other change log entries.

> Source/WebCore/ChangeLog:10
> +        No tests added as other functions in the AccessibilityRenderObject class NULL check
> +        the RenderObject::node return value.

Please explain why you're not adding a test instead of saying you're mimicking other null checks.
Comment 3 Ananta Iyengar 2011-12-27 20:03:28 PST
Created attachment 120630 [details]
proposed patch with description updated
Comment 4 Ananta Iyengar 2011-12-27 20:08:07 PST
Created attachment 120631 [details]
Patch with review comments addressed
Comment 5 chris fleizach 2011-12-27 21:15:00 PST
Comment on attachment 120631 [details]
Patch with review comments addressed

why is there no layout test for this one?
Comment 6 chris fleizach 2011-12-27 21:17:27 PST
you should be able to make this happen by inserting some html that will create an anonymous render block, and then ask for the title ui element of that anonymous element.

i'm tempted to review- this because there is no layout test
Comment 7 WebKit Review Bot 2011-12-27 21:29:19 PST
Comment on attachment 120631 [details]
Patch with review comments addressed

Clearing flags on attachment: 120631

Committed r103757: <http://trac.webkit.org/changeset/103757>
Comment 8 WebKit Review Bot 2011-12-27 21:29:23 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Ryosuke Niwa 2011-12-27 21:48:26 PST
(In reply to comment #5)
> (From update of attachment 120631 [details])
> why is there no layout test for this one?

This was causing some Chromium UI tests to fail but we didn't have a reduction in the form of a layout test. Ananta told me he's looking into creating a layout test next year.