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

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Marco Zehe (:MarcoZ)]

See bug 418368 for the discussion. Mochitest for this will be created in that bug.

Comment 1

[alexander :surkov (:asurkov)]

Marco, could you update and make full as possible your mochitests because this bug may require to change a lot of code.

Comment 2

[Marco Zehe (:MarcoZ)]

Adjusted summary. Testcase coming up.

Comment 3

[Marco Zehe (:MarcoZ)]

Attachment: Testcase

This shows that trying to get the nsIAccessibleHyperLink interface fails completely on xul:labels that have been turned into hyperlinks.

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 5

[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.

Comment 6

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 309410 [details] [diff] [review]
patch2

a1.9=beltzner

Comment 8

[alexander :surkov (:asurkov)]

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

Comment 9

[Marco Zehe (:MarcoZ)]

The test case still throws an "no interface!" test failure for me. Alex, did this work for you?

Comment 10

[alexander :surkov (:asurkov)]

Attachment: patch3

expose nsIAccessibleHyperLink unconditionally (see nsAccessible::QueryInterface) like we do html:a

Comment 11

[Marco Zehe (:MarcoZ)]

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.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 312460 [details] [diff] [review]
patch3

a=beltzner

Comment 13

[alexander :surkov (:asurkov)]

/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

Comment 14

[Marco Zehe (:MarcoZ)]

Attachment: Mochitest

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.

Comment 15

[alexander :surkov (:asurkov)]

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

Comment 16

[Marco Zehe (:MarcoZ)]

(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!

Comment 17

[Marco Zehe (:MarcoZ)]

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.

Comment 18

[Marco Zehe (:MarcoZ)]

Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041306 Minefield/3.0pre