417518 - Prevent text leaf accessibles from implementing advanced interfaces

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

It's weird that text leafs are being used as IAccessibleText. I see it in this
crash:
http://crash-stats.mozilla.com/report/index/7f4d174f-da72-11dc-b877-001a4bd43ef6

We should stop that from happening.

This is a simpler fix for that than bug 372131, because we still do need some behavior from nsLinkableAccessible in text leaf accessibles. Just changing what QI can do is a good fix for Firefox 3.

Comment 1

[Aaron Leventhal]

Attachment: Make QI do the right thing for text leaf objects

Comment 2

[Aaron Leventhal]

Marco, a try server build is underway. It has the name FixQIonTextLeaves417518.

I just want to make sure it doesn't break basic page navigation (both in or out of virtual buffer). It should be pretty obvious if this causes any problems. Please check form controls with <label>'s as well as ordinary text.

Comment 3

[Aaron Leventhal]

We also need to make sure that triggering links works, and that they are reported as visited/unvisited.

Comment 4

[Marco Zehe (:MarcoZ)]

(In reply to comment #2)
> I just want to make sure it doesn't break basic page navigation (both in or out
> of virtual buffer). It should be pretty obvious if this causes any problems.
> Please check form controls with <label>'s as well as ordinary text.


OK on all counts!

(In reply to comment #3)
> We also need to make sure that triggering links works, and that they are
> reported as visited/unvisited.

Yep also still fine!

Question: Won't this break Orca in some way if they rely on getTextAtOffset, or is this not implemented by text leaf nodes anyway?

Comment 5

[Aaron Leventhal]

Orca doesn't even see the text leaf nodes. It won't affect them at all. We only expose those on Windows for backwards-compat with older Windows screen readers. Orca sees the text via the text interface on the container elements.

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 303295 [details] [diff] [review]
Make QI do the right thing for text leaf objects


>-NS_IMPL_ISUPPORTS_INHERITED0(nsTextAccessible, nsLinkableAccessible)
>+// Make sure we don't support text or other irrelevant interfaces.
>+// We have nsLinkableAccessible in our inheritance chain as a convenience in order to
>+// get link actions and states on the text accessibles. Windows screen readers expect that.
>+NS_IMPL_ISUPPORTS_INHERITED2(nsTextAccessible, nsAccessNode, nsIAccessible, nsPIAccessible)

why nsAccessNode? Do we loose nsIAccessibleHyperLink because nsLinkableAccessible implements getURI()?

Comment 7

[Aaron Leventhal]

That's right. We don't want to support nsIAccessibleHyperLink on a text leaf. We only want to support it on elements that are embedded in text.

Comment 8

[Aaron Leventhal]

Surkov, I answered your question in comment 7.

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 303295 [details] [diff] [review]
Make QI do the right thing for text leaf objects

r=me

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 303295 [details] [diff] [review]
Make QI do the right thing for text leaf objects

a=beltzner for 1.9