300778 - Expose elements with "onclick" handlers

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

Elements with "onclick" handlers should be exposed in the MSAA tree.

If they don't have a role already we should give them a role which
differentiates them from other things.

We should make doDefaultAction() work for these elements.

Comment 1

[parente]

What state, name, and value should be reported for an accessible with role of 
onclick? Maybe value should be the value of the onclick attribute?

Comment 2

[Aaron Leventhal]

(In reply to comment #1)
> What state, name, and value should be reported for an accessible with role of 
> onclick? Maybe value should be the value of the onclick attribute?

Just inherit from nsAccessibleWrap and get the default behavior.
Name will be nothing unless there is a title attribute.
It doesn't need a value -- the AT can't make sense of the JavaScript anyway.
There won't need to be any additional states on top of the ones implemented by
default.

Let's file a spinnoff bug for the next version. Instead of "onclick" we'll use
something like "eventcontainer" and provide an API for getting a list of events
and firing them.

Comment 3

[parente]

Currently, I'm inheriting from nsLinkableAccessible to get all the machinery for 
DoAction and other methods. Works well. Is that OK?

Comment 4

[Aaron Leventhal]

(In reply to comment #3)
> Currently, I'm inheriting from nsLinkableAccessible to get all the machinery for 
> DoAction and other methods. Works well. Is that OK?

Seems okay. I don't know if we should be using STATE_LINKED or keep that as a
separate concept. If that's the case is it good enough to inherit from
nsAccessibleWrap and just copy over the doAction stuff?

Comment 5

[parente]

Attachment: Exposes a default action "click" for elements with onclick

See test case also attached to this bug.

Comment 6

[parente]

Attachment: Onclick test case

Test case HTML document.

Comment 7

[Aaron Leventhal]

Comment on attachment 189899 [details] [diff] [review]
Exposes a default action "click" for elements with onclick

r+ conditional on the following changes:
get rid of NS_ERROR_NOT_IMPLEMENTED since we do implement these methods now.
Just use NS_OK and NS_ERROR_INVALID_ARG.
2. return 0 actions when there are none
3. Use arg names like aNumActions instead of _retval. That's something we're
fixing as we go.
4. Get rid of the redundant line in DoAction:
nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));

Comment 8

[Aaron Leventhal]

Comment on attachment 189899 [details] [diff] [review]
Exposes a default action "click" for elements with onclick

Marking r+ as conditional to the changes I mentioned.

Comment 9

[parente]

Attachment: Fixes problems from first review

Comment 10

[[e-mail]]

Comment on attachment 189923 [details] [diff] [review]
Fixes problems from first review

>+           content->HasAttr(kNameSpaceID_None, nsAccessibilityAtoms::onclick) ||
This worries me slightly, but I suppose the majority of click handlers are set
in onclick attributes...

>+    *aNumActions = 1;
>+  }
>+  *aNumActions = 0;
Did you mean to have this line before the condition, or an else, or...
sr=me with this fixed.

Comment 11

[Aaron Leventhal]

We'll have to deal with event handlers set via addEventListener() at a later
time, when we also expand beyond onclick.

Comment 12

[parente]

Attachment: Fixed if/else problem

Comment 13

[Aaron Leventhal]

Checking in src/base/nsAccessibilityAtomList.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityAtomList.h,v  <-- 
nsAccessibilityAtomList.h
new revision: 1.28; previous revision: 1.27
done
Checking in src/base/nsAccessibilityService.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityService.cpp,v  <-- 
nsAccessibilityService.cpp
new revision: 1.149; previous revision: 1.148
done
Checking in src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.161; previous revision: 1.160
done

Comment 14

[Aaron Leventhal]

*** Bug 296242 has been marked as a duplicate of this bug. ***