Trays without headings throw a JavaScript error when the non-existent heading text is accessed

I had a look at this issue. The biggest issue was figuring out what to say when a tray had no heading. I considered falling back on the anchor text, however that doesn't seem very consistent, so I settled on "Tray opened." and "Tray closed.".

I have attached a patch to get the ball rolling, but if anyone has any feedback on what the fallback dictation should be, I can look at the patch again.

2: 2141417-trays-without-headings-javascript-error-2.patch queued for re-testing.

Not sure how to test it..

You can test it by adding a log to "Drupal.announce" and then opening and closing the toolbar. Everything that the aural view announces will be outputted to the console.

7: 2141417-trays-without-headings-javascript-error-7.patch queued for re-testing.

1. +++ b/core/modules/toolbar/js/views/ToolbarAuralView.js
   @@ -44,13 +44,14 @@
   +      var action = (tray === null ? 'closed' : 'opened');
   

   Strange use of parentheses here.

2. +++ b/core/modules/toolbar/js/views/ToolbarAuralView.js
   @@ -44,13 +44,14 @@
   +        text = Drupal.t('Tray "@tray" '  + action + '.', {'@tray': trayNameElement.textContent});
   ...
   +        text = Drupal.t('Tray ' + action + '.');
   

   Never use string concatenation for translation. That breaks translations. See https://api.drupal.org/api/drupal/core%21includes%21bootstrap.inc/functi....

I have attached two patches which look at the given feedback. In patch b, I have translated the 'closed' or 'opened' string before passing it in as an argument to the later translate function. Not sure if this is the right approach, however I feel that those two string would need translating at some point?

1. +++ b/core/modules/toolbar/js/views/ToolbarAuralView.js
   @@ -44,13 +44,13 @@
   +      var action = Drupal.t(tray === null ? 'closed' : 'opened');
   

   Don't use expression inside Drupal.t.

2. +++ b/core/modules/toolbar/js/views/ToolbarAuralView.js
   @@ -44,13 +44,13 @@
   +        var text = Drupal.t('Tray "@tray" @action.', {'@tray': trayName, '@action' : action});
   ...
   +        var text = Drupal.t('Tray @action.', {'@action' : action});
   

   Please move "var text" to top :)

Moved "var text" to top.

Else on a new line please :)

+++ b/core/modules/toolbar/js/views/ToolbarAuralView.js
@@ -43,14 +43,15 @@
+        action = tray === null ? 'closed' : 'opened',

null ? Drupal.t('closed') : Drupal.t('opened')

:)

Change from comment #19.

Still need the else on a new line, and while we're at it keep the one var per variable please.

Have a look at the rest of the core JS files please.

So sorry that this had to sit here for well over six months without review :(

Still applies cleanly, and still works great. Thanks!

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed e25acf2 and pushed to 8.0.x. Thanks!