Closed Bug 421066 Opened 12 years ago Closed 12 years ago

Implement all nsIAccessibleHyperLink methods for XUL:label elements that are used as links.

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: MarcoZ, Assigned: surkov)

References

Details

(Keywords: access)

Attachments

(4 files, 1 obsolete file)

See bug 418368 for the discussion. Mochitest for this will be created in that bug.
Marco, could you update and make full as possible your mochitests because this bug may require to change a lot of code.
Status: NEW → ASSIGNED
Adjusted summary. Testcase coming up.
Summary: Implement all missing nsIAccessibleHyperLink methods for XUL:label elements that are used as links. → Implement all nsIAccessibleHyperLink methods for XUL:label elements that are used as links.
Attached file Testcase
This shows that trying to get the nsIAccessibleHyperLink interface fails completely on xul:labels that have been turned into hyperlinks.
Attached patch patch (obsolete) — Splinter Review
Attachment #308820 - Flags: review?(ginn.chen)
Comment on attachment 308820 [details] [diff] [review]
patch

+already_AddRefed<nsIAccessible>
+nsLinkableAccessible::GetActionAccessible()

+  nsCOMPtr<nsIDOMNode> actionNode(do_QueryInterface(mActionContent));
+  if (!actionNode || mDOMNode == actionNode)
+    return nsnull;

Please add a comment here, explain why mDOMNode should not be actionNode


Call GetAccessibleInWeakShell in nsLinkableAccessible::CacheActionContent() caused a stack overflow.
Please fix it.
Attachment #308820 - Flags: review?(ginn.chen) → review-
Attached patch patch2Splinter Review
Attachment #308820 - Attachment is obsolete: true
Attachment #309410 - Flags: review?(ginn.chen)
Attachment #309410 - Flags: review?(ginn.chen) → review+
Attachment #309410 - Flags: approval1.9?
Comment on attachment 309410 [details] [diff] [review]
patch2

a1.9=beltzner
Attachment #309410 - Flags: approval1.9? → approval1.9+
Checking in accessible/src/base/nsBaseWidgetAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsBaseWidgetAccessible.cpp,v  <--  nsBaseWidgetAccessible.cpp
new revision: 1.69; previous revision: 1.68
done
Checking in accessible/src/base/nsBaseWidgetAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsBaseWidgetAccessible.h,v  <--  nsBaseWidgetAccessible.h
new revision: 1.27; previous revision: 1.26
done
Checking in accessible/src/html/nsHTMLAreaAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLAreaAccessible.cpp,v  <--  nsHTMLAreaAccessible.cpp
new revision: 1.33; previous revision: 1.32
done
Checking in accessible/src/html/nsHTMLAreaAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLAreaAccessible.h,v  <--  nsHTMLAreaAccessible.h
new revision: 1.18; previous revision: 1.17
done
Checking in accessible/src/html/nsHTMLLinkAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLLinkAccessible.cpp,v  <--  nsHTMLLinkAccessible.cpp
new revision: 1.26; previous revision: 1.25
done
Checking in accessible/src/html/nsHTMLLinkAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLLinkAccessible.h,v  <--  nsHTMLLinkAccessible.h
new revision: 1.23; previous revision: 1.22
done
Checking in accessible/src/xul/nsXULTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULTextAccessible.cpp,v  <--  nsXULTextAccessible.cpp
new revision: 1.35; previous revision: 1.34
done
Checking in accessible/src/xul/nsXULTextAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULTextAccessible.h,v  <--  nsXULTextAccessible.h
new revision: 1.22; previous revision: 1.21
done


Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 423603
The test case still throws an "no interface!" test failure for me. Alex, did this work for you?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch3Splinter Review
expose nsIAccessibleHyperLink unconditionally (see nsAccessible::QueryInterface) like we do html:a
Attachment #312460 - Flags: review?(marco.zehe)
Comment on attachment 312460 [details] [diff] [review]
patch3

Yes this works as expected, and also does not cause other labels to become links, either. I'll attach an updated Mochitest to this bug soon.
Attachment #312460 - Flags: review?(marco.zehe) → review+
Attachment #312460 - Flags: approval1.9?
Comment on attachment 312460 [details] [diff] [review]
patch3

a=beltzner
Attachment #312460 - Flags: approval1.9? → approval1.9+
/cvsroot/mozilla/accessible/src/xul/nsXULTextAccessible.cpp,v  <--  nsXULTextAccessible.cpp
new revision: 1.36; previous revision: 1.35
done
Checking in accessible/src/xul/nsXULTextAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULTextAccessible.h,v  <--  nsXULTextAccessible.h
new revision: 1.23; previous revision: 1.22
done

Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Attached patch MochitestSplinter Review
This Mochitest runs all supported interface method calls on two labels: One is a label with text embedded between the tags, the other has  a value attribute. and one label that has not been made a link is being tested to make sure it doesn't get undesired link-related states by any checkin.
Attachment #315537 - Flags: review?(surkov.alexander)
Comment on attachment 315537 [details] [diff] [review]
Mochitest

// Activate accessibility, otherwise events aren't fired.

nit: what events do you want to listen?

otherwise it's good, r=me
Attachment #315537 - Flags: review?(surkov.alexander) → review+
(In reply to comment #15)
> (From update of attachment 315537 [details] [diff] [review])
> // Activate accessibility, otherwise events aren't fired.
> 
> nit: what events do you want to listen?

Oops, Copy & Paste error. Will remove the part from the comma onwards. :-) Thanks for the catch!
Checking in accessible/tests/mochitest/Makefile.in;
/cvsroot/mozilla/accessible/tests/mochitest/Makefile.in,v  <--  Makefile.in
new revision: 1.14; previous revision: 1.13
done
RCS file: /cvsroot/mozilla/accessible/tests/mochitest/test_nsIAccessibleHyperLink.xul,v
done
Checking in accessible/tests/mochitest/test_nsIAccessibleHyperLink.xul;
/cvsroot/mozilla/accessible/tests/mochitest/test_nsIAccessibleHyperLink.xul,v  <--  test_nsIAccessibleHyperLink.xul
initial revision: 1.1
done

This completes this bugfix.
Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041306 Minefield/3.0pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.