Closed Bug 428248 Opened 16 years ago Closed 16 years ago

Implement tests for nsIAccessibleHyperText interface

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: MarcoZ, Assigned: MarcoZ)

References

(Blocks 1 open bug)

Details

(Keywords: access, dev-doc-complete)

Attachments

(1 file, 1 obsolete file)

Note to self: Am using the same document as for the nsIAccessibleHyperLink interface, tests for all kinds of link types.
Attached patch Patch (obsolete) — Splinter Review
1. Renamed "links" attribute to "linkCount".
2. Renamed "index" parameter to "linkIndex" on the getLink method.
3. Added/updated documentation.
4. Added tests.
Attachment #314825 - Flags: review?(surkov.alexander)
Blocks: 418054
Attached patch Missed a fileSplinter Review
Missed accessible/src/atk/nsAccessibleWrap.cpp in previous patch, otherwise identical.
Attachment #314825 - Attachment is obsolete: true
Attachment #314851 - Flags: review?(surkov.alexander)
Attachment #314825 - Flags: review?(surkov.alexander)
Comment on attachment 314851 [details] [diff] [review]
Missed a file

>+  /**
>+   * Returns the number of links contained within this hypertext object.
>+   */
>+  readonly attribute long linkCount;

we should decide is there here 's'. Many methods has 's' and it looks correct with me.

>   /*
>-   * Return the link index at this character index.
>-   * Return value of -1 indicates no link at that index.
>+   * Returns the link index at the given character index.
>+   * Each link is an embedded object representing exactly 1 character within
>+   * the hypertext.
>+   *
>+   * @param charIndex  the 0-based character index.
>+   *
>+   * @returns long  0-based link's index.
>+   * A return value of -1 indicates no link is present at that index.
>    */
>   long getLinkIndex (in long charIndex);

ideally to have the same rules through a11y module code. It seems often we don't use space between the method name and ().

>+  <script type="application/javascript">
>+    var gParaAcc

not sure "para" is nice name if you meant "paragraph". What does "para" mean? :)
Attachment #314851 - Flags: review?(surkov.alexander)
Attachment #314851 - Flags: review+
Attachment #314851 - Flags: approval1.9?
Comment on attachment 314851 [details] [diff] [review]
Missed a file

a1.9=beltzner
Attachment #314851 - Flags: approval1.9? → approval1.9+
(In reply to comment #3)
> (From update of attachment 314851 [details] [diff] [review])
> >+  /**
> >+   * Returns the number of links contained within this hypertext object.
> >+   */
> >+  readonly attribute long linkCount;
> we should decide is there here 's'. Many methods has 's' and it looks correct
> with me.

Yes, this is correct if the whole sentence was "This method returns...". I prefer to keep it that way.

> >   long getLinkIndex (in long charIndex);
> ideally to have the same rules through a11y module code. It seems often we
> don't use space between the method name and ().

Yep will fix that upon checkin.

> >+  <script type="application/javascript">
> >+    var gParaAcc
> not sure "para" is nice name if you meant "paragraph". What does "para" mean?
> :)

Will change that to gParagraphAcc. :-) Thanks!
(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 314851 [details] [diff] [review] [details])
> > >+  /**
> > >+   * Returns the number of links contained within this hypertext object.
> > >+   */
> > >+  readonly attribute long linkCount;
> > we should decide is there here 's'. Many methods has 's' and it looks correct
> > with me.
> 
> Yes, this is correct if the whole sentence was "This method returns...". I
> prefer to keep it that way.

Not sure we talk about the same thing. I meant the name of method: linkCount vs linksCount, for example, we have anchorsCount, relationsCount, targetsCount, cellsCount and etc but childCount, characterCount, selectionCount.
Checking in accessible/public/nsIAccessibleHyperText.idl;
/cvsroot/mozilla/accessible/public/nsIAccessibleHyperText.idl,v  <--  nsIAccessibleHyperText.idl
new revision: 1.7; previous revision: 1.6
done
Checking in accessible/src/html/nsHyperTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHyperTextAccessible.cpp,v  <--  nsHyperTextAccessible.cpp
new revision: 1.117; previous revision: 1.116
done
Checking in accessible/src/atk/nsAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/atk/nsAccessibleWrap.cpp,v  <--  nsAccessibleWrap.cpp
new revision: 1.110; previous revision: 1.109
done
Checking in accessible/src/atk/nsMaiInterfaceHypertext.cpp;
/cvsroot/mozilla/accessible/src/atk/nsMaiInterfaceHypertext.cpp,v  <--  nsMaiInterfaceHypertext.cpp
new revision: 1.10; previous revision: 1.9
done
Checking in accessible/src/msaa/CAccessibleHypertext.cpp;
/cvsroot/mozilla/accessible/src/msaa/CAccessibleHypertext.cpp,v  <--  CAccessibleHypertext.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in accessible/tests/mochitest/Makefile.in;
/cvsroot/mozilla/accessible/tests/mochitest/Makefile.in,v  <--  Makefile.in
new revision: 1.12; previous revision: 1.11
done
RCS file: /cvsroot/mozilla/accessible/tests/mochitest/test_nsIAccessibleHyperText.html,v
done
Checking in accessible/tests/mochitest/test_nsIAccessibleHyperText.html;
/cvsroot/mozilla/accessible/tests/mochitest/test_nsIAccessibleHyperText.html,v  <--  test_nsIAccessibleHyperText.html
initial revision: 1.1
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
(In reply to comment #6)
> Not sure we talk about the same thing. I meant the name of method: linkCount vs
> linksCount, for example, we have anchorsCount, relationsCount, targetsCount,
> cellsCount and etc but childCount, characterCount, selectionCount.
> 
OK. Well LinksCount is sort of a double double. That's why I removed the s. We count the links, but the count itself is linkCount I believe. I agree we should be consistent. But let's file a separate bug on that. For the future, I would say all count methods get the stuff they count without an "s".
(In reply to comment #6)
> Not sure we talk about the same thing. I meant the name of method: linkCount vs
> linksCount, for example, we have anchorsCount, relationsCount, targetsCount,
> cellsCount and etc but childCount, characterCount, selectionCount.

I talked to Aaron, and he agreed that linkCount or anchorCount is better than anchorsCount or linksCount. So I left it like it is in the patch, and additionally did bug 428477.
Keywords: dev-doc-needed
Docs updated.
Eric, could you point where is the documentation hosted (I assumed it should be at http://developer.mozilla.org/en/docs/nsIAccessibleHyperText but it's not).
I must be confused about what you think needs documenting.  I added the missing attributes and renamed the functions and the like.  Was that not all that needed to be done?  That is in fact where I did the work.
(In reply to comment #12)
> I must be confused about what you think needs documenting.  I added the missing
> attributes and renamed the functions and the like.  Was that not all that
> needed to be done?  That is in fact where I did the work.
> 

But where did you add that?
I made all the changes right in the nsIAccessibleHyperText article:

http://developer.mozilla.org/en/docs/nsIAccessibleHyperLink
(In reply to comment #14)
> I made all the changes right in the nsIAccessibleHyperText article:
> 
> http://developer.mozilla.org/en/docs/nsIAccessibleHyperLink
> 

This article is right for nsIAccessibleHyperLink but it isn't right for nsIAccessibleHyperText. I believe the article http://developer.mozilla.org/en/docs/nsIAccessibleHyperText should be up to dated for this bug. Changing whiteboard status to dev-doc-needed.
OK, this article is now fixed too.  I fixed the wrong one for starters -- but it turned out to need fixing too. :)

http://developer.mozilla.org/en/docs/nsIAccessibleHyperText
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: