405552 - Crash [@ nsHyperTextAccessible::ContentToRenderedOffset ]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

When SetText occurs on something with no layout frame, we crash.

See crash reports:
http://crash-stats.mozilla.com/report/list?range_unit=weeks&query_search=signature&query_type=contains&signature=nsHyperTextAccessible%3A%3AContentToRenderedOffset(nsIFrame*%2C+int%2C+unsigned+int*)&query=ContentToRenderedOffset&range_value=1

Comment 1

[Aaron Leventhal]

Attachment: 1) Don't fire text change events if text is not rendered, 2) if null frame passed in, return a rendered offset of 0, since nothing is rendered

Comment 2

[Evan Yan]

Comment on attachment 290320 [details] [diff] [review]
1) Don't fire text change events if text is not rendered, 2) if null frame passed in, return a rendered offset of 0, since nothing is rendered

It looks OK, except

> NS_IMETHODIMP nsDocAccessible::Destroy()
> {
>+  nsresult rv = Shutdown();
>+
>   if (mWeakShell) {
>+    // Remove from the cache after Shutdown(), so that Shutdown() procedures
>+    // can find the doc or root accessible in the cache if they need it
>     gGlobalDocAccessibleCache.Remove(static_cast<void*>(mWeakShell));
>   }
>-  return Shutdown();
>+  return rv;
> }
> 
This change seems about some other bug, isn't it?

|this| is the doc or root acccessible, I can't think of a case that we need to find it in the cache. And nsAccessNode::Shutdown() will set mWeakShell to null.

Comment 3

[Aaron Leventhal]

Sorry, that was from a different patch.

Comment 4

[Aaron Leventhal]

Attachment: Correct patch

Comment 5

[Aaron Leventhal]

Attachment: Correct patch