Summary: | AX: com.apple.WebKit.WebContent crashed at WebCore: WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||
Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aboxhall, apinheiro, commit-queue, dmazzoni, jcraig, jdiggs, mario, samuel_white, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
chris fleizach
2014-11-19 18:13:53 PST
Created attachment 241928 [details]
patch
Comment on attachment 241928 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=241928&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197 > + if (m_renderer && ancestorsOfType<RenderMenuList>(*m_renderer).first()) You early returned on !m_renderer, so I don't think you need this extra check, unless any of the previous calls cause any side effect on the renderer (which should not be the case) (In reply to comment #2) > Comment on attachment 241928 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241928&action=review > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197 > > + if (m_renderer && ancestorsOfType<RenderMenuList>(*m_renderer).first()) > > You early returned on !m_renderer, so I don't think you need this extra > check, unless any of the previous calls cause any side effect on the > renderer (which should not be the case) I think there is a chance that m_renderer may be set to nil by something else in the method (descendantOfBarrenChildren())? It's possible that going up the render tree is causing that. I don't have any evidence of that and I've never seen it happen, but the crash report leaves open that interpretation. do you think we should keep both checks just in case? Comment on attachment 241928 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=241928&action=review >>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197 >>> + if (m_renderer && ancestorsOfType<RenderMenuList>(*m_renderer).first()) >> >> You early returned on !m_renderer, so I don't think you need this extra check, unless any of the previous calls cause any side effect on the renderer (which should not be the case) > > I think there is a chance that m_renderer may be set to nil by something else in the method (descendantOfBarrenChildren())? It's possible that going up the render tree is causing that. I don't have any evidence of that and I've never seen it happen, but the crash report leaves open that interpretation. > > do you think we should keep both checks just in case? I guess that the "fearful me" agree on that it would perhaps be worth leaving the two checks, just in case. Anyway, as this is an speculative fix, either option (to leave both checks or only one) will be a gamble, so I think I lean more towards the safest one. Comment on attachment 241928 [details] patch Clearing flags on attachment: 241928 Committed r176484: <http://trac.webkit.org/changeset/176484> All reviewed patches have been landed. Closing bug. |