559761 - make <input type="number"> accessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Manage invalid state, expose nsIAccessibleValue

Comment 1

[alexander :surkov (:asurkov)]

changed summary by accident, sorry for spam

Comment 2

[alexander :surkov (:asurkov)]

per HTML spec (https://dvcs.w3.org/hg/html-api-map/raw-file/tip/Overview.html) we need to expose ROLE_SYSTEM_SPINBUTTON role.

Also see open HTML spec bug https://www.w3.org/Bugs/Public/show_bug.cgi?id=13562

Comment 3

[Jonathan Watt [:jwatt]]

Assuming the patch for bug lands, the patch for this bug should be sure to back out that patch.

Comment 4

[Jonathan Watt [:jwatt]]

C&P fail. Comment 3 is referring to bug 927326.

Comment 5

[alexander :surkov (:asurkov)]

marking it blocking bug a11y q4 bug since bug 635240 seems breaks basic accessibility we have by freshing the layout for the control.

Comment 6

[Jonathan Watt [:jwatt]]

The implementation that's currently in mozilla-central should be complete enough to work on a patch for this now. The dom.forms.number pref just needs to be flipped to true.

Comment 7

[Jonathan Watt [:jwatt]]

Just pinged surkov about this, and he said he will aim to take a look this week.

Here's a quick summary of the essential things to know: input type=number> contains a native anonymous tree that looks like this:

  <input type=number>
    <div>                - outer wrapper with "display:flex" by default
      <input type=text>  - text input field
      <div>              - spinner box wrapping up/down arrow buttons
        <div>            - spin up (up arrow button)
        <div>            - spin down (down arrow button)

The anonymous text field grandchild can take focus for the number input. If the a11y code looks at the focused element and it is a text field, it needs to be careful to check HTMLInputElement::ownerNumberControl to see if what it's dealing with is actually a number control and not a text control.

Comment 8

[Jonathan Watt [:jwatt]]

Also note that the pref is on for v28, so hopefully this can be uplifted. I'm very much hoping to keep it on since Tom's Hardware's Browser Grand Prix puts a fair bit of focus on <input type=number> and there's a reasonable chance v28 will be the next version they test.

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 10

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8344953 [details] [diff] [review]
patch

>+++ b/layout/forms/nsNumberControlFrame.cpp
>+#ifdef ACCESSIBILITY
>+#include "nsAccessibilityService.h"
>+#endif

mozilla/a11y/AccTypes.h should be enough right?

Comment 11

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/21ecf45dd590

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/21ecf45dd590

Comment 13

[Jonathan Watt [:jwatt]]

Thank you!

Comment 14

[Jonathan Watt [:jwatt]]

Comment on attachment 8344953 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: <input type=number> not accessible in v28
Testing completed (on m-c, etc.): landed m-i, merged to m-c
Risk to taking this patch (and alternatives if risky): low and early
String or IDL/UUID changes made by this patch: none

We're early in the cycle so hopefully this is fine to uplift. Probably neded to keep <input type=number> on in v28.

Comment 15

[Marco Zehe (:MarcoZ)]

So, Jonathan, we do want this in Aurora (Mozilla 28), right?

Comment 16

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Watt [:jwatt] from comment #14)
> Bug caused by (feature/regressing bug #): N/A

Bug 344616 and its dependencies if you need one to blame.

(In reply to Marco Zehe (:MarcoZ) from comment #15)
> So, Jonathan, we do want this in Aurora (Mozilla 28), right?

Yes, which is why I requested approval.

Comment 17

[Benjamin Kerensa [:bkerensa]]

Comment on attachment 8344953 [details] [diff] [review]
patch

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

Looks good we will go ahead approve this for uplift.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4d297d712792

Comment 19

[u279076]

Given in-testsuite coverage this fix will not be manually verified by QA. If you believe this warrants extra QA attention please nominate for testing by removing this whiteboard tag and adding the verifyme keyword. Please also provide any details you have that may inform our testing.