1088804 - NVDA screen reader: Does not read the contents of the cursor using Scratchpad

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect, P3)

Description

[DongHui]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20141024030200



Actual results:

Originally reported as NVDA issue ticket: http://community.nvda-project.org/ticket/4471
It is difficult to use NVDA Scratchpad. When you run the Scratchpad:
1. NVDA Says that the blank line. 
2. NVDA When you move the cursor does not read(ex: character, word, sentence, lineNumber and etc).

Comment 1

[James Teh [:Jamie]]

It seems that the textarea in the Scratchpad is fake in some way; it isn't actually used to expose all of the text. I'm guessing it's just for caret/selection/pasting. The problem is that this is terrible for accessibility, since you can't actually cursor through the text. I can't really comment as to how this could be fixed, as I don't really understand how the Scratchpad works nor why it was coded this way. I'm guessing it has something to do with line numbers.

Comment 2

[Marco Zehe (:MarcoZ)]

Let's move this to the proper component, then. Joe, any info you can provide on this?

Comment 3

[Joe Walker [:jwalker]]

(In reply to Marco Zehe (:MarcoZ) from comment #2)
> Let's move this to the proper component, then. Joe, any info you can provide
> on this?

CodeMirror (as used by Scratchpad) is at lease basically accessible. Maybe recent updates broke that. needinfo? Brian.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #3)
> (In reply to Marco Zehe (:MarcoZ) from comment #2)
> > Let's move this to the proper component, then. Joe, any info you can provide
> > on this?
> 
> CodeMirror (as used by Scratchpad) is at lease basically accessible. Maybe
> recent updates broke that. needinfo? Brian.

So, text input in CodeMirror works, but accessibility support for cursor movement does not (see Bug 919711).  I did some research about this last year and wrote about it here: https://bgrins.github.io/codemirror-accessible/.  Copying some points from there:

* Text based input works, as the input is being added directly into the hidden textarea
* Selection works some of the time, as it populates the textarea with the current selection. It fails when the selection is over 100 lines or 1000 characters. In this case it replaces the character with a - and marks cm.display.inaccurateSelection for cleanup later
* Cursor movement does not work. This is because the textarea is usually left empty, cleared out on regular intervals
Entering / exiting the editor does not work. Although you can generally tab into the editor without a mouse, leaving the editor is tricky and I've had to resort to mouse usage to do this. Compare the following behavior with the normal textarea at the bottom of the page. The tab button's behavior is overriden as you would expect with a code editor, which makes it hard to leave the field.

I have a fix but it's too hacky to land in CM (it uses a timeout to allow NVDA time to see the changes): https://github.com/bgrins/codemirror-accessible/compare/df4df2366fe254f3ff528e27928af6eae6b9baf5...master#diff-17ed72d8617eb30a518f39b4e72a2033L1446.  I'm not sure if the CM implementation is exactly the same as when I last looked, but basically what we need from NVDA is some way to notify a change has happened after running:

input.value = "foo";
input.setSelectionRange(1, 1);
// Clearing here will cause NVDA to not notify of the cursor movement.  If we wait to clear in a timeout it does notify.
// If there was some way to indicate to NVDA that a change has happened that would be good.
input.value = "";

Marco, could you please check if the situation has improved on the main demo page of http://codemirror.net/, I'd be more than happy to update to the latest version of CM if it improves the accessibility situation.

Comment 5

[Marco Zehe (:MarcoZ)]

Nope, it has not. The textarea on that page is just as unresponsive as ever, and the code itself is in HTL *below* the textarea. This stuff isn't even contentEditable like TinyMCE or CKEditor, so keyboard focus would move in there.

David, was there not some work going on to make CodeMirror accessible? Or does there have to be a special version of it to be used? I was never really involved with this work, so don't know anything myself. :(

Comment 6

[Brian Grinstead [:bgrins]]

Here is what Marijn (the author of CM) said in a thread discussing the research from https://bgrins.github.io/codemirror-accessible/:

> The thing about a random timeout to make the NVDA tool notice this,
> and the uncertainty about other tools, gives me the feeling that
> there's going to be more needed to have something that broadly works
> with various screen readers.

> But I'd be open to an option that modifies the behavior of both
> resetInput and readInput, in such a way that the resetting is no
> longer necessary (and the context is stably present in the textarea),
> provided that it was implemented cleanly and works on more than a
> single screen reader system.

It would be good to have a conversation with NVDA and Marijn to come up with a plan for how to add accessibility support to CM in a clean way that would work across screen reader systems.  It may be best to come up with a reduced test case that simulates what CM is doing to make that easier.

My proposed solution could work if there were a way to signal to the screen readers that a selection change has happened, but I was unable to get it to detect the change unless if I waited to clear the textarea until after an arbitrary timeout.  Maybe there is some hidden event or other notification to indicate that the selection has changed that I'm not aware of.

Comment 7

[James Teh [:Jamie]]

(FYI, I'm one of the lead NVDA developers.)

(In reply to Brian Grinstead [:bgrins] from comment #4)
> I have a fix but it's too hacky to land in CM (it uses a timeout to allow
> NVDA time to see the changes):
...
> basically what
> we need from NVDA is some way to notify a change has happened...
A selection event will be fired when this occurs. However, because events are processed asynchronously, this won't achieve what you want. Even if it did, this still won't allow the user to cursor through the field by line or word, use text review (i.e. reading text without moving the system cursor) or read with braille (which relies on being able to move to the next line on demand, etc.).

Why does CodeMirror use this fake input area rather than using contentEditable?

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to James Teh [:Jamie] from comment #7)
> Why does CodeMirror use this fake input area rather than using
> contentEditable?

I think some of the history is written down here: http://codemirror.net/doc/internals.html.  It probably also does a better job explaining the current approach than I could.

Comment 9

[James Teh [:Jamie]]

Thanks. I found that link just after I posted the comment. :)

If this is how things are going to stay (no contentEditable), it's going to be extremely difficult (if not impossible) to make this properly accessible at this stage. Cursor movement and braille *might* work if the text for the previous, current and next lines is always exposed in the input area. Text review won't. However, while ugly, more experienced users could review the main content area separately, so this isn't a total show stopper.

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to James Teh [:Jamie] from comment #9)
> Thanks. I found that link just after I posted the comment. :)
> 
> If this is how things are going to stay (no contentEditable), it's going to
> be extremely difficult (if not impossible) to make this properly accessible
> at this stage. Cursor movement and braille *might* work if the text for the
> previous, current and next lines is always exposed in the input area. Text
> review won't. However, while ugly, more experienced users could review the
> main content area separately, so this isn't a total show stopper.

I think I did have a version of the patch that kept N lines of context above and below the currently selected one.  I believe this could also be used to keep the entire content within the hidden textarea by setting a sufficiently high N, but the problem is that it has to be cleared out again for accepting input.  Additionally, when text is selected the textarea got filled with just the selected text (I think to enable clipboard access).  I can't remember if we could keep the textarea full until the start of input as opposed to running this timeout that cleared it: https://github.com/bgrins/codemirror-accessible/compare/df4df2366fe254f3ff528e27928af6eae6b9baf5...master#diff-17ed72d8617eb30a518f39b4e72a2033R2494.

Marijn, do you have any ideas how to keep the previous, current, and next lines always within the native textarea element?

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Hi Marco, unfortunately I'm no more up to date than the comments in bug 919711.

Comment 12

[Marijn Haverbeke]

In the context of mobile support, where you can't get sane selection otherwise, I'm working on implementing a mode where CodeMirror does use contentEditable. This might also be a promising avenue for improving screen reader support. Basically, the idea is that modern browsers expose enough hooks (composition events, being able to intercept copy/paste) to make contentEditable viable again for a code editor. Old browsers would continue to use the current textarea hack, but for mobile browsers, and possibly (if this turns out to be stable enough) also modern desktop ones, the editor would make itself contentEditable.

This isn't far enough along yet to release it, but I'll publish the code in a few weeks or so, and will try to remember to check back here to give you a chance to test whether the feature can be used to solve the screen reader problems.

Comment 13

[Brian Grinstead [:bgrins]]

(In reply to Marijn Haverbeke from comment #12)
> In the context of mobile support, where you can't get sane selection
> otherwise, I'm working on implementing a mode where CodeMirror does use
> contentEditable. This might also be a promising avenue for improving screen
> reader support. Basically, the idea is that modern browsers expose enough
> hooks (composition events, being able to intercept copy/paste) to make
> contentEditable viable again for a code editor. Old browsers would continue
> to use the current textarea hack, but for mobile browsers, and possibly (if
> this turns out to be stable enough) also modern desktop ones, the editor
> would make itself contentEditable.
> 
> This isn't far enough along yet to release it, but I'll publish the code in
> a few weeks or so, and will try to remember to check back here to give you a
> chance to test whether the feature can be used to solve the screen reader
> problems.

Have you had a chance to progress on this new contentEditable version of CodeMirror?

Comment 14

[Marijn Haverbeke]

Wow. I was just searching through my mail for a link to this issue when your email arrived. Not sure if I should be scared.

The contentEditable implementation is in the 'mobile' branch of the CodeMirror repository at https://github.com/codemirror/CodeMirror/tree/mobile . It can be considered a beta now -- I will create a test release for it next week, and hopefully merge it into master before next month's release.

To enable the contentEditable input backend on non-mobile platforms, set the `inputStyle: "contenteditable"` option for your CodeMirror instance. I hope that eventually, this can become the default for modern browsers, but it'll need a lot more testing before I'm taking that step.

Comment 15

[Brian Grinstead [:bgrins]]

Attachment: codemirror-mobile-integration.patch

(In reply to Marijn Haverbeke from comment #14)
> Wow. I was just searching through my mail for a link to this issue when your
> email arrived. Not sure if I should be scared.

What a weird coincidence.
 
> The contentEditable implementation is in the 'mobile' branch of the
> CodeMirror repository at
> https://github.com/codemirror/CodeMirror/tree/mobile . It can be considered
> a beta now -- I will create a test release for it next week, and hopefully
> merge it into master before next month's release.
> 
> To enable the contentEditable input backend on non-mobile platforms, set the
> `inputStyle: "contenteditable"` option for your CodeMirror instance. I hope
> that eventually, this can become the default for modern browsers, but it'll
> need a lot more testing before I'm taking that step.

Thanks!  I'm attaching a patch for testing that switches our CM version over to this one.  It updates only the relevant files I see in https://github.com/codemirror/CodeMirror/compare/master...mobile, which in our case is codemirror.js and codemirror.css.

I'll send our accessibility team a binary for initial testing once the Try server reopens.

Comment 16

[Brian Grinstead [:bgrins]]

Marco, can you take a look at a build from this push [0] and see how it works from an accessibility standpoint.  The editor is using contentEditable, but may be using some special tricks to make things work. 

Specifically, do the Style Editor and/or Scratchpad play nicely with NVDA?  Are there any other changes that should be made to the editor to improve the accessibility?

[0]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9911e9c357df

Comment 17

[Marco Zehe (:MarcoZ)]

This definitely goes in the right direction! A few observations:

1. The document that contains the scratchpad or style editor should definitely get a localized aria-label with some nice text. Because what NVDA sees now is a horrible long data: URL. Look what's written under "name":

Developer info for navigator object:
name: u"data:text/html;charset=utf8,<!DOCTYPE%20html><html%20dir='ltr'>%20%20<head>%20%20%20%20<style>%20%20%20%20%20%20html,%20body%20{%20height:%20100%;%20}%20%20%20%20%20%20body%20{%20margin:%200;%20overflow:%20hidden;%20}%20%20%20%20%20%20.CodeMirror%20{%20width:%20100%;%20height:%20100%%20!important;%20line-height:%201.25%20!important;}%20%20%20%20</style>%20%20%20%20<link%20rel='stylesheet'%20href='chrome://browser/skin/devtools/common.css'>%20%20%20%20<link%20rel='stylesheet'%20href='chrome://browser/content/devtools/codemirror/codemirror.css'>%20%20%20%20<link%20rel='stylesheet'%20href='chrome://browser/content/devtools/codemirror/dialog.css'>%20%20%20%20<link%20rel='stylesheet'%20href='chrome://browser/content/devtools/codemirror/mozilla.css'>%20%20</head>%20%20<body%20class='theme-body%20devtools-monospace'></body></html>"
role: ROLE_GROUPING
states: STATE_READONLY, STATE_FOCUSABLE
isFocusable: True
hasFocus: False
Python object: <NVDAObjects.Dynamic_GroupboxMozillaIAccessible object at 0x05B1E110>
Python class mro: (<class 'NVDAObjects.Dynamic_GroupboxMozillaIAccessible'>, <class 'NVDAObjects.IAccessible.Groupbox'>, <class 'NVDAObjects.IAccessible.mozilla.Mozilla'>, <class 'NVDAObjects.IAccessible.IAccessible'>, <class 'NVDAObjects.window.Window'>, <class 'NVDAObjects.NVDAObject'>, <class 'baseObject.ScriptableObject'>, <class 'baseObject.AutoPropertyObject'>, <type 'object'>)
description: u''
location: (294, 679, 1340, 345)
value: None
appModule: <'firefox' (appName u'firefox', process ID 1316) at address 5f91490>
appModule.productName: u'Nightly'
appModule.productVersion: u'38.0a1'
TextInfo: <class 'NVDAObjects.IAccessible.IA2TextTextInfo'>
windowHandle: 394364L
windowClassName: u'MozillaWindowClass'
windowControlID: 0
windowStyle: 399441920
windowThreadID: 6140
windowText: u'Ab sofort ist die Nutzung eigener Domains m\xf6glich | mailbox.org - Nightly'
displayText: u''
IAccessibleObject: <POINTER(IAccessible2) ptr=0xd98334c at 5ab57b0>
IAccessibleChildID: 0
IAccessible event parameters: windowHandle=394364L, objectID=-4, childID=-570626304
IAccessible accName: u"data:text/html;charset=utf8,<!DOCTYPE%20html><html%20dir='ltr'>%20%20<head>%20%20%20%20<style>%20%20%20%20%20%20html,%20body%20{%20height:%20100%;%20}%20%20%20%20%20%20body%20{%20margin:%200;%20overflow:%20hidden;%20}%20%20%20%20%20%20.CodeMirror%20{" (truncated)
IAccessible accRole: ROLE_SYSTEM_GROUPING
IAccessible accState: STATE_SYSTEM_READONLY, STATE_SYSTEM_FOCUSABLE, STATE_SYSTEM_VALID (1048640)
IAccessible accDescription: u''
IAccessible accValue: None
IAccessible2 windowHandle: 394364
IAccessible2 uniqueID: -570626304
IAccessible2 role: ROLE_SYSTEM_GROUPING
IAccessible2 states: IA2_STATE_OPAQUE (1024)
IAccessible2 attributes: u'margin-left:0px;text-align:start;text-indent:0px;margin-right:0px;tag:body;class:theme-body devtools-monospace;margin-top:0px;margin-bottom:0px;display:block;explicit-name:true;'

2. When inserting a tab stop, a span with class cm-tab is inserted into the current paragraph, causing NVDA and other screen readers to see another paragraph. Screen readers reduce their view of the world to this smallest piece of info. So what needs to happen here is that these indentations get an aria-label that tells the user that this is an indentation they landed on, and that they must press RightArrow to view the actual text on that line. aria-label can be used because it will not display anything, but screen readers will pick it up. I tested this by inserting an aria-label into such an indendation span and tested what NVDA would say. I used a text like "Indentation. Press Right Arrow to view the actual text."

Note that this second thing can only be observed if the file viewed contains actual tab stops instead of space characters. I found it while viewing a CSS file in Style Editor from https://mailbox.org. When we open ScratchPad and press Tab, we insert a couple of actual space characters, which does not make the cm-tab span show up.

Other than those, I am very happy with where this is headed! The line numbers are not spoken by default, but can be found via NVDA object navigation. This is good because otherwise they would introde on the user experience too much.

Jamie, any thoughts from your end?

Comment 18

[Marijn Haverbeke]

Is there any way we can use aria attributes to have the tab spans behave like the space spans (being ignored rather than having it read a phrase about indentation? (I imagine that would get on the user's nerves in a file with lots of tabs -- it also isn't accurate for tabs that aren't at the start of the line).

Comment 19

[Marco Zehe (:MarcoZ)]

(In reply to Marijn Haverbeke from comment #18)
> Is there any way we can use aria attributes to have the tab spans behave
> like the space spans (being ignored rather than having it read a phrase
> about indentation? (I imagine that would get on the user's nerves in a file
> with lots of tabs -- it also isn't accurate for tabs that aren't at the
> start of the line).

You can add role="presentation" to those spans instead, that would remove them and set focus to the actual paragraph containing the line. Or at least the screen reader's view of it. The line will then be read, and an indentation is seen on a refreshable braille display (not with speech, though). But yeah it can be made to ignore those tab stop spans.

Comment 20

[Marijn Haverbeke]

So a `node.setAttribute("role", "presentation")` when generating a tab placeholder should work?

There's a similar issue with non-printing characters (try inserting, for example, a non-breaking space). These are currently shown as red dots with a "title" attribute describing what they are. Should I give those an "aria-label" attribute mirroring the title, or does the title already cause the right thing to happen?

Comment 21

[Marco Zehe (:MarcoZ)]

(In reply to Marijn Haverbeke from comment #20)
> So a `node.setAttribute("role", "presentation")` when generating a tab
> placeholder should work?

Yes!

> There's a similar issue with non-printing characters (try inserting, for
> example, a non-breaking space). These are currently shown as red dots with a
> "title" attribute describing what they are. Should I give those an
> "aria-label" attribute mirroring the title, or does the title already cause
> the right thing to happen?

No, please make an aria-label. Title is a very flaky attribute to use for screen readers (and many other reasons besides mousing over things).

Comment 22

[Marijn Haverbeke]

I've added the attributes and updated the mobile branch to contain the relevant patch.

Comment 23

[Brian Grinstead [:bgrins]]

Attachment: codemirror-mobile-integration.patch

Updated with latest code from upstream mobile branch and added an aria-label to the iframe that contains the editor.

Marco / Jamie, can you confirm that it is working as expected?  Here is a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b49a60a27a8.

Comment 24

[Marco Zehe (:MarcoZ)]

Changes confirmed. :)

Comment 25

[Brian Grinstead [:bgrins]]

Going to handle the upgrade to the new version of CodeMirror that includes this feature in Bug 1132557.  The release notes consider it 'experimental' so we should do some thorough testing before toggling it on.

Comment 26

[Marco Zehe (:MarcoZ)]

So, can this bug be closed then if the new CodeMirror version is integrated?

Comment 27

[Brian Grinstead [:bgrins]]

(In reply to Marco Zehe (:MarcoZ) from comment #26)
> So, can this bug be closed then if the new CodeMirror version is integrated?

We haven't upgraded yet - Bug 1132557 is opened for this.  In that bug we will upgrade to the latest version, and then in this bug we can flip on the contentEditable mode.

Comment 28

[James Teh [:Jamie]]

Sorry I haven't gotten around to responding to this before now.

(In reply to Marco Zehe (:MarcoZ) from comment #17)
> 2. When inserting a tab stop, a span with class cm-tab is inserted into the
> current paragraph, causing NVDA and other screen readers to see another
> paragraph. Screen readers reduce their view of the world to this smallest
> piece of info.
Note that this will change once NVDA has proper support for Firefox rich text editing. Rather than focus being yanked around every time the caret moves into a new accessible, NVDA will have a more unified view of the text. I'm not sure exactly how this will impact this particular case.

> So what needs to happen here is that these indentations get
> an aria-label that tells the user that this is an indentation they landed
> on,
aria-label is currently read because NVDA treats this "paragraph" as if it were focused. However, this will change as per the above. Still, using aria-label like this is interesting. We might need to see if we can come up with a way to use aria-label even with this more unified view of the text.

Comment 29

[Brian Grinstead [:bgrins]]

(In reply to James Teh [:Jamie] from comment #28)
> Sorry I haven't gotten around to responding to this before now.
> 
> (In reply to Marco Zehe (:MarcoZ) from comment #17)
> > 2. When inserting a tab stop, a span with class cm-tab is inserted into the
> > current paragraph, causing NVDA and other screen readers to see another
> > paragraph. Screen readers reduce their view of the world to this smallest
> > piece of info.
> Note that this will change once NVDA has proper support for Firefox rich
> text editing. Rather than focus being yanked around every time the caret
> moves into a new accessible, NVDA will have a more unified view of the text.
> I'm not sure exactly how this will impact this particular case.

Is there a bug to track this progress in NVDA?

Comment 30

[James Teh [:Jamie]]

Yes, in our tracker:
http://community.nvda-project.org/ticket/1668

Comment 31

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

CodeMirror has an "inputStyle" option now which can be used to try "contenteditable" mode.

Comment 32

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Scratchpad triage, filter on "ghostbusterzzz"

Comment 33

[James Teh [:Jamie]]

The hack implemented in bug 919711 makes this reasonably accessible. (I just tested Scratchpad in Nightly.)