[AccessFu] Expose whether the input is a search, url, tel, etc.

RESOLVED FIXED in mozilla27

Status

()

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

People

(Reporter: yzen, Assigned: yzen)

Tracking

Trunk
mozilla27
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

4 years ago
Right now there is no semantic information about the type of the input that is encountered. It would be very useful to distinguish between all of them.

Comment 1

4 years ago
this issue was discussed a lot in the past, the primary argument was if sighted users can't distinguish these controls then why would AT users need that.

Comment 2

4 years ago
Oh but they can on mobile! If these types are used on mobile, as soon as they get focus, different keyboard types come up that sighted users immediately recognise. We would just put that information into the spoken semantic for the blind user to give them advance notice that the touch keyboard that will come up is not the regular one, but might have some variations. I think this argument no longer counts for mobile.

Comment 3

4 years ago
nice. ideas who to expose those via a11y API?

Comment 4

4 years ago
I thought we already expose the type object attribute?
(Assignee)

Comment 5

4 years ago
Additionally, developers often use other afordances like placeholders that give an idea for the input format.

Comment 6

4 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #4)
> I thought we already expose the type object attribute?

I don't recall it

(In reply to Yura Zenevich [:yzen] from comment #5)
> Additionally, developers often use other afordances like placeholders that
> give an idea for the input format.

placeholder are exposed as name and if not then should be exposed as description
(Assignee)

Comment 7

4 years ago
Created attachment 817658 [details] [diff] [review]
924896 patch
Attachment #817658 - Flags: review?(eitan)

Comment 8

4 years ago
Comment on attachment 817658 [details] [diff] [review]
924896 patch

>+    let entry = aAccessible.DOMNode.QueryInterface(Ci.nsIDOMElement);
>+    let typeName = entry.getAttribute('type');
>+    if (!typeName) {
>+      return;
>+    }

This retrieval should live in the C++ layer for HTML inputs and add the attribute value to the attributes collection so you can query the attribute on the accessible directly in the code above.

This will make sure that NVDA and other assistive technologies can access this info as well.

>+text           =       text

I believe this is overkill. Text is the default type for a text field and does not need to have its type announced again. The others are fine, since they deviate from the regular.
Attachment #817658 - Flags: feedback-
Comment on attachment 817658 [details] [diff] [review]
924896 patch

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

I agree with marco that the type should be exposed in the accessible attributes. So r-ing on that account.

::: accessible/src/jsat/OutputGenerator.jsm
@@ +212,5 @@
> +    if (aRoleStr !== 'entry') {
> +      return;
> +    }
> +
> +    let entry = aAccessible.DOMNode.QueryInterface(Ci.nsIDOMElement);

Is the QueryInterface really necessary?

::: dom/locales/en-US/chrome/accessibility/AccessFu.properties
@@ +74,5 @@
> +email          =       e-mail
> +search         =       search
> +tel            =       telephone
> +text           =       text
> +url            =       URL

I think these should  prefixed with something like inputType (e.g. inputTypeEmail)
Attachment #817658 - Flags: review?(eitan) → review-
(Assignee)

Comment 10

4 years ago
Created attachment 818246 [details] [diff] [review]
924896 part 1
Attachment #818246 - Flags: review?(surkov.alexander)
(Assignee)

Comment 11

4 years ago
Created attachment 818247 [details] [diff] [review]
924896 part 2
Attachment #817658 - Attachment is obsolete: true
Attachment #818247 - Flags: review?(eitan)
Attachment #818247 - Flags: feedback?(marco.zehe)
Comment on attachment 818246 [details] [diff] [review]
924896 part 1

>+      testAttrs("email", {"type" : "email"}, true);
>+      testAttrs("tel", {"type" : "tel"}, true);
>+      testAttrs("url", {"type" : "url"}, true);
>+      testAttrs("search", {"type" : "search"}, true);

Nit: Please keep these in alphabetical order. The actual markup in the below hunk is correct, only this list should have "search" moved before "tel".
Comment on attachment 818247 [details] [diff] [review]
924896 part 2

Just one nit: You could group the utterances on one line each and not split up the second like you do in the test. May make it a bit more readable. Other than that, looks great!
Attachment #818247 - Flags: feedback?(marco.zehe) → feedback+

Comment 14

4 years ago
I think type object attribute is not self descriptive, I'd say xml-roles should fit better.

Jamie, what's your opinion?
Flags: needinfo?(jamie)
why not just add new roles?

Comment 16

4 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #15)
> why not just add new roles?

we could since we don't need to care about backward compatibility on XPCOM level, but if IA2/ATK would need an object attribute then it'd be more work for us. On the other hand since those all are text entries  then role diversity should add some complexity in AT processing. What I'd really like to have is roles chain (osx's role and subrole analogue), xml-roles is best approximation of it we have today.

Marco, what do they do in Mac world?
(In reply to alexander :surkov from comment #16)
> (In reply to Trevor Saunders (:tbsaunde) from comment #15)
> > why not just add new roles?
> 
> we could since we don't need to care about backward compatibility on XPCOM
> level, but if IA2/ATK would need an object attribute then it'd be more work

or we could add roles to them...

> for us. On the other hand since those all are text entries  then role
> diversity should add some complexity in AT processing. What I'd really like

on the other hand if they process attribute to get the same information puting it in the role might remove some processing an if we're really lucky attribute getting all together.

> to have is roles chain (osx's role and subrole analogue), xml-roles is best
> approximation of it we have today.

for xpcom stuff we could have  that if we wanted / cared I guess.

Comment 18

4 years ago
Jamie, ping?

Comment 19

4 years ago
I agree this should be exposed.

I certainly don't think this should be exposed as the role, as that would break backwards compatibility. Also, the control's purpose and functionality is still the same. The input type is essentially a constraint and doesn't really constitute an entirely different control.

I'm a bit reluctant to use xml-roles, but then I've always felt that attribute was a bit ugly. Personally, I'd prefer a separate object attribute; e.g. text-input-type or similar.
Flags: needinfo?(jamie)

Comment 20

4 years ago
(In reply to James Teh [:Jamie] from comment #19)

> I'm a bit reluctant to use xml-roles, but then I've always felt that
> attribute was a bit ugly. Personally, I'd prefer a separate object
> attribute; e.g. text-input-type or similar.

taking into account an example
<input type="search" role="search">
where AT need to differentiate these as Marco said then xml-roles doesn't work

I'm fine with text-input-type. Concerns?
> I certainly don't think this should be exposed as the role, as that would
> break backwards compatibility. Also, the control's purpose and functionality

to some degree sure, but we certainly do change the roles of things to make the role more correct from time to time.

> is still the same. The input type is essentially a constraint and doesn't
> really constitute an entirely different control.

arguably links are just text with an added action and styling.

exactly how different does something need to be to get a new role?

> I'm a bit reluctant to use xml-roles, but then I've always felt that
> attribute was a bit ugly. Personally, I'd prefer a separate object
> attribute; e.g. text-input-type or similar.

I tend to think attributes are a little ugly period, but other than curiousity about why you think what you do I don't know that I care much.

Comment 22

4 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #21)
> > I certainly don't think this should be exposed as the role, as that would
> > break backwards compatibility. Also, the control's purpose and functionality
> to some degree sure, but we certainly do change the roles of things to make
> the role more correct from time to time.
That's true. However, I suspect this would break quite a lot of cases. A change like that is always a tradeoff between gains and losses. I feel the gains don't justify the losses in this case.

> arguably links are just text with an added action and styling.
That's true, but the action is a fairly significant difference. That said, we differentiate between table headers and cells, when a header is really just a special aspect of a cell. :)

> exactly how different does something need to be to get a new role?
I'm not at all claiming this is clear-cut. For me, the most significant point is the gains versus losses concerning backwards compatibility.

I forgot to mention that another possibility is IAccessible2::extendedRole. It's never been used before, but that doesn't mean we can't start now. :)
(Assignee)

Comment 23

4 years ago
Created attachment 820796 [details] [diff] [review]
924896 part 1 v2
Attachment #818246 - Attachment is obsolete: true
Attachment #818246 - Flags: review?(surkov.alexander)
Attachment #820796 - Flags: review?(surkov.alexander)
(Assignee)

Comment 24

4 years ago
Created attachment 820797 [details] [diff] [review]
924896 part 2 v2
Attachment #818247 - Attachment is obsolete: true
Attachment #818247 - Flags: review?(eitan)
Attachment #820797 - Flags: review?(eitan)
Attachment #820797 - Flags: feedback?(marco.zehe)

Updated

4 years ago
Attachment #820797 - Flags: feedback?(marco.zehe) → feedback+

Comment 25

4 years ago
(In reply to James Teh [:Jamie] from comment #22)

> I forgot to mention that another possibility is IAccessible2::extendedRole.
> It's never been used before, but that doesn't mean we can't start now. :)

what is your preference? attributes are universal but they felt overloaded. atk doesn't have this possibility though

Comment 26

4 years ago
Comment on attachment 820796 [details] [diff] [review]
924896 part 1 v2

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

::: accessible/src/generic/Accessible.cpp
@@ +1371,5 @@
> +  // especially for mobile.
> +  nsAutoString type;
> +  if (Role() == roles::ENTRY &&
> +    mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::type, type))
> +      nsAccUtils::SetAccAttr(attributes, nsGkAtoms::textInputType, type);

I'd rather put it under HTMLTextFieldAccessible class
Comment on attachment 820797 [details] [diff] [review]
924896 part 2 v2

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

This looks great. The OutputGenerator could use a general refactor, IMHO. But this works well within this scope.
Attachment #820797 - Flags: review?(eitan) → review+
(Assignee)

Comment 28

4 years ago
Created attachment 821260 [details] [diff] [review]
924896 part 1 v3
Attachment #820796 - Attachment is obsolete: true
Attachment #820796 - Flags: review?(surkov.alexander)
Attachment #821260 - Flags: review?(surkov.alexander)

Updated

4 years ago
Attachment #821260 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 29

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/229a8f20d8ed
https://hg.mozilla.org/integration/mozilla-inbound/rev/f59777bbd950
Comment on attachment 821260 [details] [diff] [review]
924896 part 1 v3

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

::: content/base/src/nsGkAtomList.h
@@ +2180,5 @@
>  GK_ATOM(tableCellIndex, "table-cell-index")
>  GK_ATOM(tablist, "tablist")
>  GK_ATOM(textAlign, "text-align")
>  GK_ATOM(textIndent, "text-indent")
> +GK_ATOM(textInputType, "text-input-type")

Do we care how long these strings are?
(In reply to David Bolter [:davidb] from comment #30)
> Comment on attachment 821260 [details] [diff] [review]
> 924896 part 1 v3
> 
> Review of attachment 821260 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/base/src/nsGkAtomList.h
> @@ +2180,5 @@
> >  GK_ATOM(tableCellIndex, "table-cell-index")
> >  GK_ATOM(tablist, "tablist")
> >  GK_ATOM(textAlign, "text-align")
> >  GK_ATOM(textIndent, "text-indent")
> > +GK_ATOM(textInputType, "text-input-type")
> 
> Do we care how long these strings are?

obviously putting a 1m string in libxul would be really bad, but not more than any other literal string.
(In reply to Trevor Saunders (:tbsaunde) from comment #31)
> (In reply to David Bolter [:davidb] from comment #30)
> > Comment on attachment 821260 [details] [diff] [review]
> > 924896 part 1 v3
> > 
> > Review of attachment 821260 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: content/base/src/nsGkAtomList.h
> > @@ +2180,5 @@
> > >  GK_ATOM(tableCellIndex, "table-cell-index")
> > >  GK_ATOM(tablist, "tablist")
> > >  GK_ATOM(textAlign, "text-align")
> > >  GK_ATOM(textIndent, "text-indent")
> > > +GK_ATOM(textInputType, "text-input-type")
> > 
> > Do we care how long these strings are?
> 
> obviously putting a 1m string in libxul would be really bad, but not more
> than any other literal string.

I meant my question to be about how long the object attributes string is getting. Probably best to discuss on IRC/email I guess.
Backed out for test failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f59777bbd950&jobname=mochitest-other
eg: https://tbpl.mozilla.org/php/getParsedLog.php?id=29607023&tree=Mozilla-Inbound

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/47a9389f5474
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cdfbe1d54ba1
(Assignee)

Comment 34

4 years ago
Created attachment 821941 [details] [diff] [review]
924896 part 2 v3

Fixing a failing test when password would have a text input type but not an entry role.
Attachment #820797 - Attachment is obsolete: true
Attachment #821941 - Flags: review?(eitan)
Attachment #821941 - Flags: review?(eitan) → review+
(Assignee)

Comment 35

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=0e240d90f484
(Assignee)

Comment 36

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/87a32e0b05ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/53746157d20f
https://hg.mozilla.org/mozilla-central/rev/87a32e0b05ba
https://hg.mozilla.org/mozilla-central/rev/53746157d20f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27

Updated

4 years ago
Depends on: 932158
Depends on: 934737
You need to log in before you can comment on or make changes to this bug.