Closed
Bug 428248
Opened 16 years ago
Closed 16 years ago
Implement tests for nsIAccessibleHyperText interface
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
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)
12.55 KB,
patch
|
surkov
:
review+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
Note to self: Am using the same document as for the nsIAccessibleHyperLink interface, tests for all kinds of link types.
Assignee | ||
Comment 1•16 years ago
|
||
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)
Assignee | ||
Comment 2•16 years ago
|
||
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 3•16 years ago
|
||
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 4•16 years ago
|
||
Comment on attachment 314851 [details] [diff] [review] Missed a file a1.9=beltzner
Attachment #314851 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 5•16 years ago
|
||
(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!
Comment 6•16 years ago
|
||
(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.
Assignee | ||
Comment 7•16 years ago
|
||
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
Assignee | ||
Comment 8•16 years ago
|
||
(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".
Assignee | ||
Comment 9•16 years ago
|
||
(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
Comment 11•16 years ago
|
||
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).
Comment 12•16 years ago
|
||
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.
Comment 13•16 years ago
|
||
(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?
Comment 14•16 years ago
|
||
I made all the changes right in the nsIAccessibleHyperText article: http://developer.mozilla.org/en/docs/nsIAccessibleHyperLink
Comment 15•16 years ago
|
||
(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.
Keywords: dev-doc-complete → dev-doc-needed
Comment 16•16 years ago
|
||
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
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•