Problem/Motivation

We had a huuuuuuuuuuuuuuge discussion on accessibility of the inline editing capability from Spark at the Toronto Accessibility sprint. Here's the outcome of that:

In general, when approaching accessibility, there are two personas you're concerned with:

1) Keyboard-only users, who can see what's going on but don't have a pointing device.
2) Screenreader users, who are keyboard-only users, but without the benefit of being able to see what's going on.

So we need to think of this in terms of what both of those audiences would want to do, without stabbing themselves in the face 500 times. :) So let's step through the flow. I've *bolded the things that need to change, and/or need special accessibility attention.

  1. Browsing through the site, arriving at a page, spotting a typo (nothing special accessibility-wise).
  2. Tab through to the "Edit" toolbar item at the top (we're assuming here that we're moving to Jesse's toolbar design rather than how Edit module works right now) and hit Enter.
  3. This expands the tray, which should show a *Quick edit mode toggle (which should become a "button" type with Aria style of "pressed" when it is toggled on ... we should remove "View" here because then the difference between Edit module's "View" and Node module's "View" is unclear and confusing). Hit Enter to turn it on.
  4. This overlays a white sheen as it does now, and outlines the things on the page that are editable. *We need to style these differently than they are currently, e.g. a dashed outline. Right now they look like all elements have focus and they do not. This is confusing.
  5. *Tabbing from this point only tabs between editables (and then eventually back to the toolbar + quick edit toggle). So you should not be able to tab around to links inside the node body, for example, or on non-editable sidebars, when "Quick edit mode" is on.
  6. *As I tab through in a screenreader, I'll hear something like: "Title field. Editable. Press Enter to edit." (TODO: How to implement this?)
  7. If I hit Enter, the window opens up with the contents of whatever in it. Here there are three different behaviours, depending on the type of field:
    • For a "show me a widget" field, *put the cursor on the first focusable element. Currently, this is the Save button (well, currently, it's the Label but *the Label should not be focusable, but probably eventually this will be the Cancel button when we move Save to the top (?). This is important so that screen reader users can discover that Save/Cancel exists.
    • For a "content editable" field (plain textfield), or a WYSIWYG field, the cursor should be positioned in just before the first character of the text. However, if you click into the field with a pointing device, it should continue to position the cursor directly where you clicked, as it does now.
  8. If I shift-tab from the place I'm at now, *the dialog closes, and I'm back to my pre-existing editable having focus. A screen reader user should hear something like *Title field dialog closed. as this happens. This is equivalent to clicking somewhere off the window for a pointing device.
  9. Similarly, if I tab through all possible fields, without changing any of the field values, *the dialog closes, and the next editable (e.g. Body) now has focus. Once again, a screen reader user hears something like *Title field dialog closed. as this happens. This is equivalent to clicking somewhere off the window for a pointing device.
  10. If I tab through into one of the fields on the window and edit its contents, and then tab out (either by shift+tabbing several times or tab-tabbing several times) I will get a modal dialog that warns me about unsaved changes, and either a "Discard changes" or "Save" option (same as it works now — assuming the modal is accessible, this is fine accessibility-wise). *We'll have to revisit this if/when we move to a single save button + draft revisions option.
  11. For just editing, you tab through, change values, tab through, change values. To save, you either tab once more and invoke the modal dialog that warns you about dropping your changes, OR you shift+tab to go back up to the save button and click it. I noted that this seems a bit odd to rely on the "exception" behaviour for inline editing, and questioned whether we should repeat the save/cancel buttons at the bottom again, but Everett said this is the kind of detail we should test later on once it's committed and tweak from there.

A few technical challenges:

- ARIA has roles like "has pop-up" but not like "is editable." We'll need to figure out a way to explain to people what's happening.
- Need to be able to make things that are normally focusable non-focusable so you can tab between editables.We'll have to leave those elements there with the presentation role, but strip out all of their children dynamically.
- Everett also said that since Drupal 7, where we had to be careful about using more advanced accessibility features such as ARIA roles, but people will have had 3-4 years to upgrade their technology. So in the same way we don't support IE 6 and 7 in D8, we can say we don't support screen readers that don't support ARIA roles, as long as we're dealing with "advanced" editing like this, and not the basic "Edit tab on the Node page" editing.

Proposed resolution

We'll address this issue in three ways:

  1. Make the activation and deactivation of edit mode a one-click operation.
  2. Provide appropriate feedback through auditory means to the user so that the state of the page is expressed sufficiently.
  3. Create a logical tabbing structure in edit mode so that non-sited and keyboard-only users aren't traversing through unrelated parts of the page.

Remaining tasks

#1851046: Edit mode should be activated when clicking the Edit toolbar tab
#1851092: When entering edit mode, inform users through the aria-live aural update interface about the state of page
#1851074: Isolate tabbable fields/links to editable fields and Edit module control components when edit mode is enabled
#1851144: Use <button> for elements that are buttons

User interface changes

TBD.

API changes

None.

Comments

+1

Project:Edit» Drupal core
Issue summary:View changes

x

Project:Drupal core» Edit

Wow, this is awesome and extremely helpful! Thank you! :)

It'll definitely be an interesting challenge to make all of this work. At the same time, we'll gain a lot of very useful accessibility experience, so that we can make it more accessible from the start in the future :)

I'd *love* to jump in and start making the first dent in making Edit fully accessible (the very first tiny dent was made back at DrupalCon Munich, by setting role="button" where necessary). I do fully understand how it should work from a screenreader user point of view. I do not understand at all yet how it should work from an implementer's point of view.

I'd very much appreciate links to useful references to get started on this. If you say "you'll have to read the entire ARIA spec to be able to productively implement this", that's fine, but I'd like to avoid spending days reading things that won't help me after all.

Below is a point-by-point list to explain my main questions.

  • Point 2: will be provided by #1137920: Fix toolbar on small screen sizes and redesign toolbar for desktop, and is blocked on that being committed.
  • Points 3 & 4: @tkoleary is designing this.
  • Points 5 & 6: I only have an extremely vague idea of how to implement this. Tabindexes come to mind, but more useful references would be most welcome.
  • Point 7: for the "form" editor, Edit should give focus to the "Save" button, but for the "direct" editors (contentEditable or WYSIWYG), Edit should give focus to the text, specifically it should put the cursor before the first word. This distinction makes little sense to me, so please help me understand why this is necessary. Why shouldn't we either always give focus to the "Save" button or always give focus to first editable thingie, whether that's a form field or a piece of text?
  • Points 8 & 9: this ties back to my remark for points 5 & 6. Except that this sounds even more complex: we must make sure that the screen reader speaks a message that is invisible to non-screenreader users. How would that work?
  • Points 10 & 11: no questions for now.

And finally, a general question: if I make sure all of this works well in VoiceOver, will this also work in JAWS etc?

Project:Edit» Drupal core
Issue summary:View changes

x

Issue summary:View changes

added the issue template

Issue summary:View changes

added 1851046, 1851092, 1851074

Project:Drupal core» Edit
Issue tags:+a11y

added tag a11y

All remaining tasks in this first pass at creating an aural, keyboard-focused interface for the Edit module have been completed and checked into the 8.x-1.x branch of the Edit module.

You can test the results of this work at the following demo site:

user: tester
password: tester
URL: http://edit.qemistry.us/

Further work on the aural UI will require some refactoring of the Edit module JavaScript. What we would to do now is collect feedback on the current attempt to make this UI accessible. This feedback will form the plan for the second effort after refactoring is completed.

I personally am on a steep learning curve for implementing aural UIs. I've been using ChromeVox for testing and also as a user agent for non-development web browsing. I'm coming to understand the level of effort required to build a solid aural UI; that it's really building another interface to the features of this module. We all want to make this module usable by everyone. Feedback and implementation suggestions or code are truly encouraged.

Project:Edit» Drupal core
Version:8.x-1.x-dev» 8.x-dev
Component:Accessibility» edit.module
Priority:Critical» Major
Issue tags:+accessibility

Drupal standard profile should be accessible, it looks like we are making good progress. Moving it around - so we can get some eyes on this, now that Edit is in core.

Priority:Major» Critical

Priority:Critical» Normal

Unless proven otherwise, I don't believe there are any outstanding a11y problems with inline editing that would qualify as "major". (EDIT: or critical, for that matter. :P) But if I'm incorrect on that count, happy to have this moved back.

Does that link still work? I got "Server not found"

No, but the good news is you can just download 8.x and install it now. :D

Status:Active» Needs review

Marking as NR, to hopefully start getting some much-needed a11y reviews.

@mgifford: see what webchick said; if you want to try it quickly, then go to http://simplytest.me/project/drupal/8.x.

Much easier to do now.

So in my local test I couldn't access the WYSIWYG without a mouse. Tabbing just jumps right over the CKEditor elements.

Like discussions with BU Editor earlier if there is a way to access it, it needs to be discoverable.
http://drupal.org/node/1306760#comment-5107904

Assigned:Unassigned» jessebeach
Issue tags:+CKEditor in core

#12: That would be an a11y bug in Edit, not in the WYSIWYG editor (i.e. CKEditor) :) Assigning to jessebeach because she's the best person to answer that. I'll make sure we coordinate with the CKEditor developers because this might lead us to tricky ARIA territory.

Also tagging accordingly.

Issue tags:+Spark

.

Status:Needs review» Postponed
Issue tags:+sprint

Points 1–4 have been implemented already.

Points 5–9 are now possible to implement thanks to #1913086: Generalize the overlay tabbing management into a utility library. This should only be implemented as soon as #1979784: Factor Create.js and VIE.js out of the Edit module is completed.

Points 10 & 11 are being addressed at #1678002: Edit should provide a usable entity-level toolbar for saving fields.

Issue tags:-a11y

Tags

Title:Accessibility plan for Edit moduleMake in-place editing keyboard or aurally accessible
Status:Postponed» Active
Issue tags:+a11y

Ok, This is back in active development. #1678002: Edit should provide a usable entity-level toolbar for saving fields is now matured enough to build on.

Issue tags:+atag

adding atag tag.

Priority:Normal» Major

This seems at least major.

Title:Make in-place editing keyboard or aurally accessibleMake in-place editing keyboard and aurally accessible
Status:Active» Needs review
StatusFileSize
new25.59 KB
PASSED: [[SimpleTest]]: [MySQL] 57,627 pass(es).
[ View ]

I tried to cover all the major UX components in this patch: keyboard navigation, aural announcements for state changes, correct aria role and state markup, and tabbing management for the various stages of in place editing.

Rather than take time to explain in words, I made a little video that illustrates the interaction: http://www.youtube.com/watch?v=0lcQj1ht6cA&feature=youtube_gdata_player

I still have to mark up and announce validation errors. That will land in an upcoming patch tomorrow.

I would love for folks who use screen readers to test this patch and let me know what works and what doesn't work for you. I'll post a link to a simplytest.me build that you can use to quickly test the feature.

Test the patch in #21 on simplytest.me.

STM seems to be down at the moment. Will try to test this later.

Has anyone been able to test out with Jaws? If not, I can jump in.

@richardgoodrow no one has tested with Jaws. I would very much appreciate your input.

Did a short test in VoiceOver on STM and it seemed to work fine.

StatusFileSize
new7.29 KB
new20.95 KB
PASSED: [[SimpleTest]]: [MySQL] 58,199 pass(es).
[ View ]

Accessibility signed off: great!

Now let's look at the code. Code review below; a whole bunch of nitpicky things I've fixed in the attached reroll.

  1. +++ b/core/misc/tabbingmanager.js
    @@ -41,7 +41,10 @@ $.extend(TabbingManager.prototype, {
    -  constrain: function (elements) {
    +  constrain: function (elements, expand) {

    New parameter, but no new docs.

  2. +++ b/core/misc/tabbingmanager.js
    @@ -51,7 +54,16 @@ $.extend(TabbingManager.prototype, {
    +    var $elements;
    +    // Find tabbable elements within the set of supplied elements if expansion
    +    // is requested.
    +    if (expand) {
    +      $elements = $(elements).find(':tabbable').addBack(':tabbable');
    +    }
    +    // Assume that a list of tabbable elements is provided.
    +    else {
    +      $elements = $(elements);
    +    }

    This means that some of the elements that we're passing in are not in fact tabbable (because they are filtered away by :tabbable), yet we want those to be tabbable anyway, right?

    Which elements are these?

  3. +++ b/core/modules/contextual/contextual.toolbar.js
    @@ -222,10 +222,13 @@ Drupal.contextualToolbar = {
    -        tabbingContext = Drupal.tabbingManager.constrain($('.contextual-toolbar-tab, .contextual'));
    -        this.model.set('tabbingContext', tabbingContext);
    -        this.announceTabbingConstraint();
    -        this.announcedOnce = true;
    +        var $tabbables = $('.contextual');
    +        if ($tabbables.length) {
    +          tabbingContext = Drupal.tabbingManager.constrain($('.contextual-toolbar-tab').add($tabbables));
    +          this.model.set('tabbingContext', tabbingContext);
    +          this.announceTabbingConstraint();
    +          this.announcedOnce = true;
    +        }

    This means that some of the elements that we're passing in are not in fact tabbable (because they are filtered away by :tabbable), yet we want those to be tabbable anyway, right?

    Which elements are these?

  4. +++ b/core/modules/edit/js/editors/formEditor.js
    @@ -63,12 +63,17 @@ Drupal.edit.editors.form = Drupal.edit.EditorView.extend({
    +    var owns = $(this.fieldModel.get('entity').get('el')).attr('aria-owns').split(' ');
    +    owns.push(id);
    +    $(this.fieldModel.get('entity').get('el')).attr('aria-owns', owns.join(' '));

    This means that some of the elements that we're passing in are not in fact tabbable (because they are filtered away by :tabbable), yet we want those to be tabbable anyway, right?

    Which elements are these?

  5. +++ b/core/modules/edit/js/views/AppView.js
    @@ -252,11 +261,16 @@ Drupal.edit.AppView = Backbone.View.extend({
    -      el: $(fieldModel.get('el')),
    +      el: fieldModel.get('el'),

    Good catch :) That was indeed unnecessary!

  6. +++ b/core/modules/edit/js/views/EditorDecorationView.js
    @@ -30,6 +31,9 @@ Drupal.edit.EditorDecorationView = Backbone.View.extend({
    +
    +    // Manage keyboard clicks.
    +    //this.$el.on('keypress.edit', this.onKeyPress.bind(this));

    This can go away I think?

  7. +++ b/core/modules/edit/js/views/EditorDecorationView.js
    @@ -141,14 +145,28 @@ Drupal.edit.EditorDecorationView = Backbone.View.extend({
    +      .prop({'tabIndex': '0'})
    +++ b/core/modules/edit/js/views/EntityToolbarView.js
    @@ -316,12 +316,17 @@ Drupal.edit.EntityToolbarView = Backbone.View.extend({
    +              'tabindex': '0'
    ...
    +              'tabindex': '0',

    Why do we have these if we have the tabbing manager?

  8. +++ b/core/modules/edit/js/views/EntityView.js
    @@ -16,15 +16,62 @@ Drupal.edit.EntityView = Backbone.View.extend({
    +    this.$el.off('.edit');

    I think this accidentally snuck in?

  9. +++ b/core/modules/edit/js/views/FieldAuralView.js
    @@ -0,0 +1,70 @@
    +    Drupal.announce(Drupal.t('Editing @aria', {'@aria': this.model.get('metadata').aria}), 'assertive');

    That's a strange string to translate, I made this a little bit nicer.

  10. +++ b/core/modules/edit/js/views/FieldAuralView.js
    @@ -0,0 +1,70 @@
    +    // @todo, announce the validation errors. And mark them correctly with
    +    // aria-invalid=true

    Still to be implemented before this can be RTBC.

Awesome fixes Wim! mgifford and I are going to do a live co-review of the patch this Friday.

#28: Just for clarity, most of the things I mentioned in the code review in #27 I did not yet fix in my reroll! The reroll only changed things I was certain of.

Status:Needs review» Needs work
StatusFileSize
new17.44 KB
new17.44 KB

Rerolled and addressed the switch to the new Drupal modal. Working on the items from Wim's feedback that were not incorporated into the issue yet.

Here is a link to launch a simplytest.me instance.

We're looking to determine if the in-place editing support for keyboard and aural access is sufficient. Evaluation questions include:

  1. Are the controls discoverable?
  2. Is the path through the tabbables acceptable?
  3. Can you edit all the fields in an entity with quick-edit and save them or cancel out successfully?
  4. What obstacles do you encounter?
  5. Do you have suggestions for improvement?

Should also add here that to test this a user needs to create a new article or page and after saving the page, click on the "Quick edit" button to test the in-place editing functionality.

StatusFileSize
new308.44 KB

Just trying to give a bit more context
What the screen looks like when you're starting a Quick edit of a document.

Mike and I found during a co-evaluation that tabbing in Firefox is broken. We weren't able to tab from a field to the entity toolbar to save changes to the field. I'm looking into this.

Also it's difficult to discover the toolbar non-visually. I'm going to have the entity toolbar be the thing that is tabbed to after exiting a recently changed field, so that next action will be to save after editing a field.

StatusFileSize
new17.46 KB

Straight reroll of #30, because the patch didn't apply anymore.

Issue summary:View changes

added 1851144

Issue summary:View changes
Issue tags:-sprint
StatusFileSize
new3.95 KB
new14.31 KB

Reroll to keep up with HEAD.

Also, tabbing between field elements no longer worked, so I fixed that.

I cannot yet speak to the Javascript code itself. However, I just installed this patch and was able to tab through all of visible fields on a node successfully with a mix of tab and enter. This is in Chrome.

In Firefox, I was able to tab between fields and select the save button to save the form. The only issue I had is that the cursor keys in the WYSIWYG didn't seem to work. That seems unrelated, though.

I'm not RTBCing because I don't know how to test the aural parts and I'm not qualified to judge the Javascript code itself. If someone can advise I can look into it.

Further testing has found other issues:

When using a multi-value longtext field, when that field is in "edit" mode, the "add another item" button does not work. Tabbing to it and selecting it or clicking the button with the mouse produces a momentary "working" icon, but no new field is added.

Additionally, it's unclear if the desired behavior is for the tabbing to cycle through only the editable fields. It is still tabbing through all links on the page, the browser location bar, etc. I don't know if that's by design or not. (In the use case we have for D7 it's probably not desirable, but I don't know from an accessibility standpoint what is "correct" here.)

#38: if there's a "working" icon, then there's an AJAX request. What is the response you're getting? That definitely used to work.

ajax.js line 551 is throwing a 404 error on path /system/ajax

Interesting.

Failed to load resource: the server responded with a status of 400 (Bad Request) http://d8.drupal.dev/system/ajax
Uncaught AjaxError:
An AJAX HTTP error occurred.
HTTP Result Code: 400
Debugging information follows.
Path: /system/ajax
StatusText: Bad Request
ResponseText: A fatal error occurred:

I reproduced the error. 400, bad request. Looking into it.

This error is happening on HEAD (8b0cb059), it's unrelated to this issue.

From WatchDog:

56  21/Jan 23:47  warning  ajax  Invalid form POST data.
57  21/Jan 23:50  notice   php   Notice: unserialize(): Error at offset 223880 of 334725 bytes in
                                  Drupal\Core\KeyValueStore\DatabaseStorageExpirable->getMultiple() (line 70 of
                                  /Users/jbeach/code/drupal/core/d8/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php).

I can get my debugger to catch right inside \Drupal\system\FormAjaxController::content() when requesting another instance of the long text field, but it only catches for a moment, then the request crumbles and returns an error. I'm still trying to determine what the actual issue is, but this type of failure is right on the edge of my debugging skills. I don't have a way to dive deeper into what's going on. Maybe we need to look at the stack trace?

I think this may be related to #2177289: In-place editing of image fields is broken, which itself is caused by AJAX forms being somehow broken.

Issue tags:+Needs reroll

The patch in #36 no longer applies and seems to need a serious re-roll.