In #504378: Auto-save draggable forms I found out that:

After dragging a block to another section, we see the message:

* The changes to these blocks will not be saved until the Save blocks button is clicked.

I don't know for sure that this is in the drupal messages box, if not we'll update the issue title/summary.

I assume that where this is happening it is done in a standard way. I also assume that other modules might be adding messages to this/some other standard container with JS.

This causes problems for users of AT (primarily screen-readers and magnifiers) as they may not notice the dynamically added message.

WAI-ARIA provides a live region, which indicates to AT that they should monitor a region of the DOM for changes. We should add this to drupal messages and / or wherever this is commonly occurring. We should add with JS, as the problem will only occur w/ JS enabled.

Files: 
CommentFileSizeAuthor
#79 tabledrag-alert-1272990-79.patch1.37 KBBarisW
#68 Alert-for-weight-change.png33.93 KBmgifford
#66 tabledrag-alert-1272990-66.patch2.63 KBdroplet
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 101,466 pass(es). View
#64 tabledrag-alert-1272990-64.patch1.44 KBmgifford
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,052 pass(es), 12,404 fail(s), and 6,141 exception(s). View
#62 tabledrag-alert-1272990-62.patch1.51 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,542 pass(es). View
#59 tabledrag-alert-1272990-59.patch2.55 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,598 pass(es). View
#54 tabledrag-alert-1272990-54.patch2.53 KBmgifford
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch tabledrag-alert-1272990-54.patch. Unable to apply patch. See the log in the details link for more information. View
#52 tabledrag-alert-1272990-52.patch2.52 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 64,596 pass(es). View
#47 tabledrag-alert-1272990-47.patch2.56 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tabledrag-alert-1272990-47.patch. Unable to apply patch. See the log in the details link for more information. View
#44 tabledrag-alert-1272990-44.patch3.65 KBjessebeach
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tabledrag-alert-1272990-44.patch. Unable to apply patch. See the log in the details link for more information. View
#35 tabledrag-alert-1272990-35.patch5.71 KBjessebeach
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tabledrag-alert-1272990-35.patch. Unable to apply patch. See the log in the details link for more information. View
#35 interdiff_32-to-35.txt3.74 KBjessebeach
#32 tabledrag-alert-1272990-32.patch2.95 KBjessebeach
PASSED: [[SimpleTest]]: [MySQL] 53,075 pass(es). View
#31 Screen Shot 2013-03-11 at 8.42.15 AM.png249.04 KBmgifford
#30 Drupal.announce-function-dragNdrop-1272990-30.patch1.01 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 53,074 pass(es). View
#26 Drupal.announce-function-dragNdrop-26.patch861 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 52,472 pass(es). View
#23 Drupal.announce-function-dragNdrop-23.patch846 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 50,951 pass(es). View
#12 aria2dragNdrop-1272990-12.patch758 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 50,781 pass(es). View

Comments

Everett Zufelt’s picture

Title: Add WAI-ARIA live region to drupal messags box » Add WAI-ARIA live region to drupal message box
Bojhan’s picture

Subscribe, this would be awesome.

Everett Zufelt’s picture

Can someone tell me where class="js" is being added to <html>? Would likely be best to add in the same spot, as part of the 'is JS enabled' test'.

Everett Zufelt’s picture

Title: Add WAI-ARIA live region to drupal message box » Add WAI-ARIA live region support for tabledrag warning message

Unfortunately this is not adding the message to a pre-existing container, but is inserting a container / message before the table:

/includes/tabledrag.js

        $(Drupal.theme('tableDragChangedWarning')).insertBefore(self.table).hide().fadeIn('slow');

...

Drupal.theme.prototype.tableDragChangedWarning = function () {
  return '<div class="tabledrag-changed-warning messages warning">' + Drupal.theme('tableDragChangedMarker') + ' ' + Drupal.t('Changes made in this table will not be saved until the form is submitted.') + '</div>';
};


Everett Zufelt’s picture

What needs to be done to resolve this is:

- An inline div needs to be inserted before the table w/ role="alert". This needs to happen when JS first modifies the table adding all the dnd handlers.

- Where the code is currently inserting the Save message before the table it needs to insert it as a child of the div added above.

Everett Zufelt’s picture

Also just found out that the warning message isn't shown if 'show row weights' is enabled.

Everett Zufelt’s picture

Title: Add WAI-ARIA live region support for tabledrag warning message » Make tabledrag warning message show when row weights are enabled, and add WAI-ARIA live region
mgifford’s picture

This sounds like a great contribution.

Bojhan’s picture

Status: Active » Needs work

Lets do this consistently in core, sounds like Everett outlined nicely what a patch would need to contain.

mgifford’s picture

Issue tags: +aria

Adding tag for WAI-ARIA.

Also, we need someone to roll the patch. Any takers?

mgifford’s picture

Issue tags: +JavaScript

Tagging... Also, tabledrag's moved to:

core/misc/tabledrag.js

mgifford’s picture

Status: Needs work » Needs review
FileSize
758 bytes
PASSED: [[SimpleTest]]: [MySQL] 50,781 pass(es). View

@Bojhan do we have lists of places where there are interactive messages like in Drag/Drop?

I'd like to make this consistent, but not sure how many places this might be done in D8.

@Everett, I'm not sure this is what you were looking at but decided to start the ball rolling by simply adding role="alert" to the warning. The "Changed" note should also be explained I think but at the very least the text would need to change.

$.extend(Drupal.theme, {
  tableDragChangedMarker: function () {
    return '<abbr class="warning tabledrag-changed" title="' + Drupal.t('Changed') + '">*</abbr>';
  },
  tableDragIndentation: function () {
    return '<div class="indentation">&nbsp;</div>';
  },
  tableDragChangedWarning: function () {
    return '<div class="tabledrag-changed-warning messages warning" role="alert">' + Drupal.theme('tableDragChangedMarker') + ' ' + Drupal.t('Changes made in this table will not be saved until the form is submitted.') + '</div>';
  }
});
mgifford’s picture

#12: aria2dragNdrop-1272990-12.patch queued for re-testing.

mgifford’s picture

Issue tags: +a11ySprint

Just needs a review it seems.

mgifford’s picture

This is a pretty simple patch to review.. Only included the addition of:
role="alert"

However, it might be better here to use aria-live="polite":
https://developer.mozilla.org/en-US/docs/Accessibility/ARIA/ARIA_Live_Re...

Either would be an improvement.

mgifford’s picture

#12: aria2dragNdrop-1272990-12.patch queued for re-testing.

Wim Leers’s picture

Note that the Edit module is already doing this! Thanks to jessebeach.

jessebeach’s picture

If this patch solves the acute issue, I say we commit it for the addition info it provides.

We've got custom implementations of aria-live in Edit and Toolbar. Frankly, any UI I touch going forward will get aria-live messaging because, well, that's what building a UI is all about - making the state of the application known.

After feature freeze I'd like to consolidate the Toolbar and Edit module area-live implementations into a common implementation under something like Drupal.inform. There's no reason to hold a quick fix for a bigger fix, though.

jessebeach’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x, thanks! Will push in a few mins.

Wonder if this is worth a backport to D7.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

mgifford’s picture

Status: Closed (fixed) » Active

Realized there's a problem with the approach taken here for Safari & IE9.

Looking forward to @jessebeach's API #1915302: Provide an API for aria-live region update announcements that module developers may leverage which should provide a standardized way to do this.

mgifford’s picture

Status: Active » Needs review
FileSize
846 bytes
PASSED: [[SimpleTest]]: [MySQL] 50,951 pass(es). View

Rough patch to use Drupal.announce function. We still need role="alert", but can't count on it to announce it to AT.

Wim Leers’s picture

Status: Needs review » Needs work
+++ b/core/misc/tabledrag.jsundefined
@@ -1244,7 +1244,9 @@ $.extend(Drupal.theme, {
+    return '<div class="tabledrag-changed-warning messages warning" role="alert">' + Drupal.theme('tableDragChangedMarker') + ' ' + changeWarning + '</div>';
+    Drupal.announce(changeWarning);

You're returning before Drupal.announce() is called; hence it won't be called :)

mgifford’s picture

Right.. Thanks Wim. Juggling a few things today and missed that obvious one. Have to do local testing too.

mgifford’s picture

Status: Needs work » Needs review
FileSize
861 bytes
PASSED: [[SimpleTest]]: [MySQL] 52,472 pass(es). View

Ok, I'm getting a bit further with this, but still getting:

Uncaught TypeError: Object #<Object> has no method 'announce' tabledrag.js:1248

I'm not sure why this isn't working Drupal.announce(changeWarning, 'assertive');

mgifford’s picture

Wim Leers’s picture

Sounds like a JS lib dependency problem: it seems Drupal.announce is loaded after the calling code.

skilip’s picture

Drupal.announce is part of drupal.js, which is loaded before tabledrag.js, so that shouldn't cause any troubles. Maybe use window.Drupal.announce() instead? Or wrap tabledrag code like this?

(function ($, Drupal, drupalSettings) {
...
})(jQuery, Drupal, drupalSettings);
mgifford’s picture

FileSize
1.01 KB
PASSED: [[SimpleTest]]: [MySQL] 53,074 pass(es). View

Well, we can try it.

mgifford’s picture

Well, the patch works in SimplyTest.me, but not for VoiceOver based on my testing.

Drupal.announce(changeWarning, 'assertive');

I'm not seeing what I'd expect from: #1915302: Provide an API for aria-live region update announcements that module developers may leverage

Screen Shot of my screen with the patch applied.

jessebeach’s picture

FileSize
2.95 KB
PASSED: [[SimpleTest]]: [MySQL] 53,075 pass(es). View

In this case, I think you added code to the tabledrag.js tableDragChangedWarning method, but this method is overridden in blocks.js, which is probably why you weren't hearing the announcement. That's my guess in any case.

mgifford, do we need Drupal.announce here? Shouldn't the role="alert" attribute be sufficient to get the warning message read? That's what I did in this patch, with some cleanup of the strings. If we really do need Drupal.announce here, let me know and I'll update it or I'm glad to review an update if you want to do it.

mgifford’s picture

It should have been enough, but in testing this in Safari with VoiceOver I realize I didn't see it. Searched found http://blog.paciellogroup.com/2012/06/html5-accessibility-chops-aria-rol...

Thought we could use the Drupal.announce approach here instead:
https://drupal.org/node/1272990#comment-7063974

It works as expected in ChromeVox. Just tested just now.

EDIT: Patch above doesn't seem to get VoiceOver to work in Safari either unfortunately.

jessebeach’s picture

It's a pity that Safari doesn't support role="alert"! Oh well, I'll put the Drupal.announce back in that you originally had.

jessebeach’s picture

Status: Needs review » Needs work
FileSize
3.74 KB
5.71 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tabledrag-alert-1272990-35.patch. Unable to apply patch. See the log in the details link for more information. View

mgifford, I made some changes to Drupal.announce to deal with the Overlay. VoiceOver and ChromeVox don't seem to be reading aria-live element changes that occur in nodes inside of iframes. So we always need to be manipulating the live element in the parent window.

Let me know if this patch works for you in Safari. The tabledrag.js warning is not quite working correctly yet. If you navigate from the Blocks page to a Content type field display page and alter the table, it will read the warning from the Blocks page. We need to be passing the text for the warning into the theme function, rather than simply including the text in the theme function.

mgifford’s picture

That worked great in Safari/VoiceOver. Thanks for re-rolling the patch.

mgifford’s picture

Where are we at with this patch? What else needs to happen before we let the bots at it.

jessebeach’s picture

Status: Needs work » Needs review

mgifford, this is the one piece we need to address:

The tabledrag.js warning is not quite working correctly yet. If you navigate from the Blocks page to a Content type field display page and alter the table, it will read the warning from the Blocks page. We need to be passing the text for the warning into the theme function, rather than simply including the text in the theme function.

jessebeach’s picture

It "works" right now, but you navigate around the Overlay, you can end up getting the wrong message across pages that use the Drag-drop warnings. Whatever the first message you get is, that's the one you'll have for the rest of your overlay session.

mgifford’s picture

Ok, thanks for the update. I was just trying to touch on some of the issues that have dropped a bit lower in the priority list over the past month.

mgifford’s picture

#35: tabledrag-alert-1272990-35.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +JavaScript, +accessibility, +aria, +a11ySprint

The last submitted patch, tabledrag-alert-1272990-35.patch, failed testing.

mgifford’s picture

@Jesse, can re-roll this patch?

jessebeach’s picture

FileSize
3.65 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tabledrag-alert-1272990-44.patch. Unable to apply patch. See the log in the details link for more information. View

This is just a straight reroll on D8:60a1055049ac218322c09b9bd753289ac39a2259. I don't have time to fix the issue I mentioned in #39 right at the moment.

mgifford’s picture

Status: Needs work » Needs review

Want to see if #44 runs into problems with the bots.

Still have to deal with #39 as @jessebeach says.

Status: Needs review » Needs work

The last submitted patch, tabledrag-alert-1272990-44.patch, failed testing.

mgifford’s picture

FileSize
2.56 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tabledrag-alert-1272990-47.patch. Unable to apply patch. See the log in the details link for more information. View

Reroll with visually-hidden...

mgifford’s picture

Status: Needs work » Needs review

Go bot.

mgifford’s picture

#47: tabledrag-alert-1272990-47.patch queued for re-testing.

mgifford’s picture

Status: Needs review » Needs work

The last submitted patch, 47: tabledrag-alert-1272990-47.patch, failed testing.

mgifford’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
2.52 KB
PASSED: [[SimpleTest]]: [MySQL] 64,596 pass(es). View

just a re-roll.

Jalandhar’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
mgifford’s picture

Status: Needs work » Needs review
FileSize
2.53 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch tabledrag-alert-1272990-54.patch. Unable to apply patch. See the log in the details link for more information. View

Thanks for finding that @Jalandhar.

jhedstrom’s picture

Patch in #54 still applies. Adding the manual testing tag.

Status: Needs review » Needs work

The last submitted patch, 54: tabledrag-alert-1272990-54.patch, failed testing.

nod_’s picture

Issue tags: +Needs reroll
mgifford’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.55 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,598 pass(es). View
mgifford’s picture

Oh ya, this should be basically identical to the last one. Just a bit of fuzz:

patching file core/misc/tabledrag.js
Hunk #1 succeeded at 1277 with fuzz 1 (offset 16 lines).
patching file core/modules/block/js/block.js
Hunk #1 succeeded at 132 (offset 1 line).

nod_’s picture

Status: Needs review » Needs work

We can drop the parent.window thing. No overlay anymore.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.51 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,542 pass(es). View

That makes it simpler. No need to modify core/misc/announce.js then, right?

droplet’s picture

+++ b/core/misc/tabledrag.js
@@ -1277,6 +1277,14 @@
+    // Announce the changes.

+++ b/core/modules/block/js/block.js
@@ -132,4 +132,16 @@
+  // Announce the changes.

redundant comments

mgifford’s picture

FileSize
1.44 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,052 pass(es), 12,404 fail(s), and 6,141 exception(s). View

Ok, I removed that line from core/modules/block/js/block.js

Status: Needs review » Needs work

The last submitted patch, 64: tabledrag-alert-1272990-64.patch, failed testing.

droplet’s picture

FileSize
2.63 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 101,466 pass(es). View

We should not replace the Drupal.theme.element globally.

Passing var in patch still not ideally way, but sadly there's limitation on Drupal.theme().

mgifford’s picture

Status: Needs work » Needs review

What does the bot think of this new patch?

mgifford’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing
FileSize
33.93 KB

I've tested this in ChromeVox & looked at the page source. Drupal.announce is being properly presented.

I think this is good to go.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/misc/tabledrag.js
@@ -779,7 +779,13 @@
+          if (!self.tableDragChangedWarningTheme) {
...
+          }

Should we just have a (documented) var on the tabledrag object which defaults to 'tableDragChangedWarning'

mgifford’s picture

Issue tags: +accessibility, +aria
jessebeach’s picture

I'm unsure why we'd insert an element with warning text in a div with role=alert and also invoke Drupal.announce. I presume this would result in the message getting announced twice. But then, role=alert doesn't have great support. So maybe it's a valid hedge.

mgifford’s picture

Issue tags: +aria-live
andrewmacpherson’s picture

Issue tags: +Drupal.announce

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed d3f88a8 on 8.3.x
    Issue #1272990 by mgifford, Everett Zufelt: Fixed Make tabledrag warning...

  • webchick committed d3f88a8 on 8.3.x
    Issue #1272990 by mgifford, Everett Zufelt: Fixed Make tabledrag warning...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andrewmacpherson’s picture

I think comment #69 is still an issue to look at?

BarisW’s picture

Status: Needs work » Needs review
Issue tags: +Dublin2016
FileSize
1.37 KB

So I tested the patch on both ChromeVox and Voiceover. In ChromeVox it works fine, in Voiceover I don't hear the message announced. Either way, I get the same results for the version with and without role=alert . I would suggest to remove the role=alert since the Drupal announce already reads the text.

andrewmacpherson’s picture

OK, that sounds good to me.

Support for aria-live="assertive" varies between different screenreaders (and role="alert" is supposed to imply aria-live="assertive").

It sounds like we have run into that problem, and we can't fix it in Drupal. Instead, by avoiding role=alert we can get the Drupal.announce() message instead.

mgifford’s picture

What else do we need to do before marking this RTBC? This sounds like the right direction to go.

Andrew, have you tested this code independently? If not, I guess that's the next step.

Bojhan’s picture

I dont fully understand why we are doing "Make tabledrag warning message show when row weights are enabled"

  • webchick committed d3f88a8 on 8.4.x
    Issue #1272990 by mgifford, Everett Zufelt: Fixed Make tabledrag warning...

  • webchick committed d3f88a8 on 8.4.x
    Issue #1272990 by mgifford, Everett Zufelt: Fixed Make tabledrag warning...
mgifford’s picture

@Bojhan I'm confused by your response.

Does the issue need a clearer description?

We need to use Drupal.announce() to tell screen readers that this text has appeared on the page.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Bojhan’s picture

Yes, but it doesn't need to be displayed right?

mgifford’s picture

I agree @Bojhan there should be no visible changes. Just WAI-ARIA.