Implementation of nsIAccessibleHyperText




17 years ago
17 years ago


(Reporter: gilbert.fang, Assigned: gilbert.fang)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(3 attachments, 2 obsolete attachments)



17 years ago
#include "nsISupports.idl"
#include "nsIAccessibleHyperLink.idl"

[scriptable, uuid(8f18d922-1dd2-11b2-82ea-829b78a44413)]
interface nsIAccessibleHyperText : nsISupports
  readonly attribute long links;

  nsIAccessibleHyperLink getLink (in long index);

  long getLinkIndex (in long charIndex);

Comment 1

17 years ago
see also bugsceri 25.


17 years ago
Blocks: 136315

Comment 2

17 years ago
Jessie will be in charge of this bug.
QA Contact: dsirnapalli →


17 years ago
Blocks: 151107

Comment 3

17 years ago
Created attachment 89710 [details] [diff] [review]

extend the nsRootAccessbile to support nsIAccessibleText

Comment 4

17 years ago
1) this code is not compact enough:
+  if (hyperlink) {
+    return PR_TRUE;
+  }
+  return PR_FALSE;

+nsresult nsRootAccessible::GetLinksFromAccNode(nsIAccessible *aAccNode,
I think you can return a PRInt32 for link number instead of returning NS_OK 
always. Same thing for GetAccNodeCharLength().

+  if (IsHyperLink(aAccNode)) {
+    links = 1;
+  }
+  if (0 == aIndex && 1 == links) {
+     return CallQueryInterface(aAccNode, _retval);
+  }
You can put those two conditions together. Because IsHyperLink(aAccNode) and (1 
== links) are equivalent in your case.

Comment 5

17 years ago
oh, kyle. Thank u for your pre-review. :-) unfortunately, I think I should
extend the nsHTMLIFrameAccessible instead of nsRootAccessible. The latter is the
really acc node for the whole content pane. :-/

Comment 6

17 years ago
Hi, kyle, as for your comments, 
#1, yes . It will  be "return hyperlink? PR_TRUE, PR_FALSE;"
#2, yes. That will make the source a little more neatly and efficiently
#3, yes. It will be like 
  if (IsHyperLink(aAccNode)) {
    links = 1; /* links is also used later
    if (0 == aIndex)
      return CallQueryInterface(aAccNode, _retval);
Kyle, you really have a eagle eye :-()

Comment 7

17 years ago
Created attachment 89773 [details] [diff] [review]
new patch(patch_bz147168_20020701_b)

1 this patch is to extend the nsHTMLIFrameAccessible instead of
nsRootAccessible to support the nsIAccessibleHyperText
2 this patch depends on the patch for bugzilla 150603(implementation of
3 this patch has been modified by kyle's comments (comments #4)
Need r=?
Attachment #89710 - Attachment is obsolete: true

Comment 8

17 years ago
Yeah, nsHTMLIFrameAccessible is the right place to implemet 
nsIAccessibleHyperText now. some nits:
in nsHTMLIFrameAccessible::GetLinksFromAccNode()
+  PRInt32 rv = 0;
+  //see whether beginning node  is a hyperlink
+  if (IsHyperLink(aAccNode)) {
+    rv = 1;
+  }
could be: PRInt32 rv = IsHyperLink(aAccNode) ? 1 : 0;
+    childLinks = GetLinksFromAccNode(child);
+    rv += childLinks;
could be rv += GetLinksFromAccNode(child); |childLinks| can be saved.

else is okay for me!

Comment 9

17 years ago
I think your fix by using
+NS_IMPL_ISUPPORTS_INHERITED2(nsHTMLIFrameAccessible, nsBlockAccessible, 
nsIAccessibleDocument, nsIAccessibleHyperText)
is more professional in XPCOM's point of view. Can it apply to 
nsHTMLIFrameRootAccessible too?
Aaron, whether the original code (NS_INTERFACE_MAP_BEGIN, 
NS_IMPL_ADDREF_INHERITED, etc.) is not fully XPCOM compliance?

Comment 10

17 years ago
Created attachment 89883 [details]
Simpel test case 

A simple test case in java script  to demo  nsIAccessibleHyperText interface.

Comment 11

17 years ago
Created attachment 89905 [details] [diff] [review]
a new patch following the comments #8(patch_bz147168_20020702_a)

Oh, Kyle's suggestions is good. But I am really  a little bewildered  whether
the code is  too concise  to make it easy to read in the future. Anyway, I
agree with kyle and make this new patch.
Attachment #89773 - Attachment is obsolete: true

Comment 12

17 years ago
For comments #9
Of course, NS_IMPL_ISUPPORTS_INHERITEDX can also be applied to
I have read the xpcom/glue/nsISupportsImpl.h and found that
NS_IMPL_ISUPPORTS_INHERITEDX actually the more convenient way to substitute the
others macros
617 #define NS_IMPL_ISUPPORTS_INHERITED2(Class, Super, i1, i2)                    \
618     NS_IMPL_QUERY_INTERFACE_INHERITED2(Class, Super, i1, i2)                  \
619     NS_IMPL_ADDREF_INHERITED(Class, Super)                                    \
620     NS_IMPL_RELEASE_INHERITED(Class, Super)                                   \

But what I want to know is whether we purify the old code in this bug or file a
new bug.  If this bug's patch in fact changes something unrelated with the
summary, it may be diffcult for others to track the bugs. How is the mozilla's
way to purify old code which is correct at all for only neatness?

Comment 13

17 years ago
Comment on attachment 89905 [details] [diff] [review]
a new patch following the comments #8(patch_bz147168_20020702_a)

Attachment #89905 - Flags: review+
Comment on attachment 89905 [details] [diff] [review]
a new patch following the comments #8(patch_bz147168_20020702_a)

Attachment #89905 - Flags: superreview+

Comment 15

17 years ago
checked in
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 16

17 years ago
Created attachment 90273 [details]
Test Cases for nsIAccessibleHyperText for HTML element
You need to log in before you can comment on or make changes to this bug.