doAction(0) in <textarea/> accessible raises error

RESOLVED FIXED in mozilla24

Status

()

Core
Disability Access APIs
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: MarcoZ, Assigned: eeejay)

Tracking

Trunk
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
STR:
1. Go to http://facebook.com and log in if you aren't already.
2. Find and double-tap the "What are you doing?" text field. Note that TalkBack speaks it as a text field and not a textarea.

Result:
1. TalkBack says "Editing", and I assume the keyboard becomes visible, too.
2. Half a second later or so, TalkBack says "Navigating", and the keyboard disappears again.

Facebook has replaced the text field with a whole set of widgets including post and cancel buttons above, and a couple of unlabeled other buttons below, a newly appeared text area where one can enter a status.

Continuing with the STR:
3. Find that text area and double-tap.

Expected: Keyboard should come up so I can enter my status.
Actual: Nothing happens.
(Reporter)

Comment 1

4 years ago
Another place where this reproduces is Twitter:
1. Go to http://mobile.twitter.com and log in.
2. Double-tap "New Tweet".
3. Find and double-tap the text area for the tweet.

Same result: Keyboard does not come up.
(Reporter)

Comment 2

4 years ago
A place where this does not reproduce is http://alpha.app.net. When logged in, the textarea to post a status can be double-tapped and the keyboard comes up.

Wild guess: contentEditable mapped to a multi-line text field via ARIA on Facebook and Twitter, versus a real textarea element on app.net?
(Assignee)

Comment 3

4 years ago
This is failing for me:
doing doAction(0) on a textarea, even when the actionCount is 1 ('activate') brings up an NS_ERROR_FAILURE for me. I'll investigate further, and come up with either a mochitest or a patch.
(Reporter)

Comment 4

4 years ago
Interesting! Other screen readers probably just set focus and don't use the actions for this type of control.
(Assignee)

Comment 5

4 years ago
Created attachment 753920 [details] [diff] [review]
Failing test case for textarea

Here is a test. Anyone could pick this up. Or I'll get to it next week.
(Assignee)

Updated

4 years ago
OS: Android → All
Hardware: ARM → All
Summary: [AccessFu] Keyboard does not come up when double-tapping the "What are you doing?" text area in Facebook → doAction(0) in <textarea/> accessible raises error
(Reporter)

Comment 6

4 years ago
CC'ing Surkov in case he has a quick idea.

Comment 7

4 years ago
because textarea isn't HTMLInputElement (http://mxr.mozilla.org/mozilla-central/source/accessible/src/html/HTMLFormControlAccessible.cpp#456)
(Assignee)

Comment 8

4 years ago
Created attachment 755058 [details] [diff] [review]
Fix activate action for <textarea/> and add action tests for text input elements.
Attachment #755058 - Flags: review?(surkov.alexander)

Comment 9

4 years ago
Comment on attachment 755058 [details] [diff] [review]
Fix activate action for <textarea/> and add action tests for text input elements.

Review of attachment 755058 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/html/HTMLFormControlAccessible.cpp
@@ +458,5 @@
>        return element->Focus();
>  
> +    nsCOMPtr<nsIDOMHTMLTextAreaElement> textArea(do_QueryInterface(mContent));
> +    if (textArea)
> +      return textArea->Focus();

I'd rather added FromContent to HTMLElement class which implements nsIDOMHTMLElement defining Focus() method (for example see http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsGenericHTMLElement.h#64)

::: accessible/tests/mochitest/actions.js
@@ +9,3 @@
>  
>  const CLICK_EVENTS = MOUSEDOWN_EVENT | MOUSEUP_EVENT | CLICK_EVENT;
> +const ALL_EVENTS = CLICK_EVENTS | COMMAND_EVENT | FOCUS_EVENT;

I'd rename it to XUL_EVENTS since "command" event belongs to XUL world. In opposite to XUL_EVENTS you can have HTML_EVENTS.

We don't have tests where we invoke an action twice (thus no focus event in second case) and probably we don't have tests where we invoke a link action that opens a new window so having FOCUS_EVENT is ALL_EVENTS works but I really wouldn't include it. If you like then you could have HTML_CONTROLS_EVENTS or simply use HTML_EVENTS | FOCUS_EVENT.

::: accessible/tests/mochitest/actions/test_controls.html
@@ +61,5 @@
> +        },
> +        {
> +          ID: "textinput",
> +          actionName: "activate",
> +          events: FOCUS_EVENT

then it's not clear ALL_EVENTS change

@@ +103,5 @@
>    </fieldset>
> +
> +  <textarea id="textarea" placeholder="What's happening?"></textarea>
> +
> +  <input id="textinput" type="text"/>

you don't need to have trailing '/' to close tag since you are in HTML file
Attachment #755058 - Flags: review?(surkov.alexander)

Updated

4 years ago
Assignee: nobody → eitan
(Assignee)

Comment 10

4 years ago
(In reply to alexander :surkov from comment #9)
> Comment on attachment 755058 [details] [diff] [review]
> Fix activate action for <textarea/> and add action tests for text input
> elements.
> 
> Review of attachment 755058 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/html/HTMLFormControlAccessible.cpp
> @@ +458,5 @@
> >        return element->Focus();
> >  
> > +    nsCOMPtr<nsIDOMHTMLTextAreaElement> textArea(do_QueryInterface(mContent));
> > +    if (textArea)
> > +      return textArea->Focus();
> 
> I'd rather added FromContent to HTMLElement class which implements
> nsIDOMHTMLElement defining Focus() method (for example see
> http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/
> nsGenericHTMLElement.h#64)
> 

I a don't understand exactly what you mean. I could use FromContent(), of nsGenericHTMLElement or nsGenericHTMLFormElement, but they do not implement or inherit from nsIDOMHTMLElement, so Focus() is not available (at least not the one that returns an nsresult).

I took the liberty of simply calling Accessible::TakeFocus(), since we don't really check the element type in the other action methods anyway. For example, ActionCount() always returns one no matter the type.

> ::: accessible/tests/mochitest/actions.js
> @@ +9,3 @@
> >  
> >  const CLICK_EVENTS = MOUSEDOWN_EVENT | MOUSEUP_EVENT | CLICK_EVEN    aName.AssignLiteral("activate");
    return NS_OK;
  }T;
> > +const ALL_EVENTS = CLICK_EVENTS | COMMAND_EVENT | FOCUS_EVENT;
> 
> I'd rename it to XUL_EVENTS since "command" event belongs to XUL world. In
> opposite to XUL_EVENTS you can have HTML_EVENTS.
> 

I renamed it to XUL_EVENTS (even though it is not related to this bug).

> We don't have tests where we invoke an action twice (thus no focus event in
> second case) and probably we don't have tests where we invoke a link action
> that opens a new window so having FOCUS_EVENT is ALL_EVENTS works but I
> really wouldn't include it. If you like then you could have
> HTML_CONTROLS_EVENTS or simply use HTML_EVENTS | FOCUS_EVENT.
> 
> ::: accessible/tests/mochitest/actions/test_controls.html
> @@ +61,5 @@
> > +        },
> > +        {
> > +          ID: "textinput",
> > +          actionName: "activate",
> > +          events: FOCUS_EVENT
> 
> then it's not clear ALL_EVENTS change

I added it to ALL_EVENTS because the name suggested it should be added :) I renamed it to XUL_EVENTS and removed it.

> 
> @@ +103,5 @@
> >    </fieldset>
> > +
> > +  <textarea id="textarea" placeholder="What's happening?"></textarea>
> > +
> > +  <input id="textinput" type="text"/>
> 
> you don't need to have trailing '/' to close tag since you are in HTML file

Right...
(Assignee)

Comment 11

4 years ago
Created attachment 756030 [details] [diff] [review]
Bug 875665 - Fix activate action for <textarea/> and add action tests for text input elements.

I hope this is better.
Attachment #755058 - Attachment is obsolete: true
Attachment #756030 - Flags: review?(surkov.alexander)

Comment 12

4 years ago
Comment on attachment 756030 [details] [diff] [review]
Bug 875665 - Fix activate action for <textarea/> and add action tests for text input elements.

Review of attachment 756030 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Attachment #756030 - Flags: review?(surkov.alexander) → review+

Comment 13

4 years ago
(In reply to Eitan Isaacson [:eeejay] from comment #10)

> > I'd rather added FromContent to HTMLElement class which implements
> > nsIDOMHTMLElement defining Focus() method (for example see
> > http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/
> > nsGenericHTMLElement.h#64)
> > 
> 
> I a don't understand exactly what you mean. I could use FromContent(), of
> nsGenericHTMLElement or nsGenericHTMLFormElement, but they do not implement
> or inherit from nsIDOMHTMLElement, so Focus() is not available (at least not
> the one that returns an nsresult).

I meant you can implement FromContent on HTMLElement class, that class implements nsIDOMHTMLElement which you need.
(Assignee)

Comment 14

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a5ab810a451
https://hg.mozilla.org/mozilla-central/rev/2a5ab810a451
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24

Updated

4 years ago
Duplicate of this bug: 868059

Updated

4 years ago
Duplicate of this bug: 868057
You need to log in before you can comment on or make changes to this bug.