241062 - link name is not supported

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Louie Zhao]

Using at-poke, no link name is displayed for HyperText because
nsHTMLLinkAccessibleWrap doesn't support :GetStartIndex and :GetEndIndex.

Comment 1

[Louie Zhao]

Attachment: patch v1

Comment 2

[Philip K. Warren]

Comment on attachment 146569 [details] [diff] [review]
patch v1

>Index: accessible/src/atk/nsHTMLLinkAccessibleWrap.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/atk/nsHTMLLinkAccessibleWrap.cpp,v
>retrieving revision 1.9
>diff -u -r1.9 nsHTMLLinkAccessibleWrap.cpp
>--- accessible/src/atk/nsHTMLLinkAccessibleWrap.cpp	10 Jan 2004 03:15:31 -0000	1.9
>+++ accessible/src/atk/nsHTMLLinkAccessibleWrap.cpp	19 Apr 2004 11:14:47 -0000
>@@ -122,6 +124,45 @@
>   return NS_OK;
> }
> 
>+nsresult nsHTMLLinkAccessibleWrap::GetLinkOffset(PRInt32* aStartOffset, PRInt32* aEndOffset)
>+{
>+  NS_ENSURE_TRUE(mTextNodes, NS_ERROR_FAILURE);
>+
>+  nsCOMPtr<nsILink> currentLink(do_QueryInterface(mLinkContent));
>+  NS_ENSURE_TRUE(currentLink, NS_ERROR_FAILURE);
>+
>+  PRUint32 index, count = 0;
>+  PRUint32 totalLength = 0, textLength = 0;
>+
>+  mTextNodes->Count(&count);
>+  for (index = 0; index < count; index++) {
>+    nsCOMPtr<nsIDOMNode> domNode(do_QueryInterface(mTextNodes->ElementAt(index)));
>+    nsCOMPtr<nsIDOMText> domText(do_QueryInterface(domNode));
>+    if (domText) {
>+      domText->GetLength(&textLength);
>+      totalLength += textLength;
>+    }

Do we expect this to fail? If we cannot get an nsIDOMText from an nsIDOMNode,
do we want to have an assertion, or at least reset textLength to zero so we do
not set invalid start/end indexes below?

>+
>+    // text node maybe a child (or grandchild, ...) of a link node
>+    nsCOMPtr<nsIDOMNode> parentNode;
>+    nsCOMPtr<nsILink> link = nsnull;
>+    domNode->GetParentNode(getter_AddRefs(parentNode));
>+    while (parentNode) {
>+      link = do_QueryInterface(parentNode);
>+      if (link)
>+        break;
>+      nsCOMPtr<nsIDOMNode> temp = parentNode;
>+      temp->GetParentNode(getter_AddRefs(parentNode));
>+    }
>+
>+    if (link == currentLink) {
>+      *aEndOffset = totalLength;
>+      *aStartOffset = totalLength - textLength;
>+      return NS_OK;
>+    }

Comment 3

[Louie Zhao]

> 
> Do we expect this to fail? If we cannot get an nsIDOMText from an nsIDOMNode,
> do we want to have an assertion, or at least reset textLength to zero so we do
> not set invalid start/end indexes below?
> 
If any elment in mTextNodes isn't Text Node, we will skip that element. This is
consistent with implementation of nsAccessibleHyperText, where "mTextNodes"
comes from.

Comment 4

[Aaron Leventhal]

Louie, what kinds of nodes might it be other than a text node? Could an image
node be in there?

Comment 5

[Louie Zhao]

(In reply to comment #4)
> Louie, what kinds of nodes might it be other than a text node? Could an image
> node be in there?

Since we use simple testcase to do debugging, I haven't found non-text node in 
mTextNodes till now. Your suggestion is reasonable, which is quite "strict
checking". But at present, I suggest to fix this bug using current solution
because it adopt the same way as used in nsAccessibleHyperText (e.g. refer to
http://lxr.mozilla.org/seamonkey/source/accessible/src/atk/nsAccessibleHyperText.cpp#144,
there are also lots of similar computing in this file).

If we should have an assertion or reset textLength to zero in case non-text node
exists in mTextNodes, we need update nsAccessibleHyperText.cpp also. I suggest
to file another bug to deal with it if it's really a problem. (Fortuately, QA
hasn't found any bug related to it till now).

Comment 6

[Louie Zhao]

Comment on attachment 146569 [details] [diff] [review]
patch v1

Henry, this bug blocks bug 246770. Can you spare some time to sr the patch?
Thanks very much.

Comment 7

[Philip K. Warren]

Louie: Can this patch be checked in to the trunk now?

Comment 8

[Louie Zhao]

Thanks for remindering. Patch checked in.