From 6eb02300787941962fcf803b3e1788ff56e4fd67 Mon Sep 17 00:00:00 2001
From: Harisha Rajam Swaminathan
 <35213866+harisha-swaminathan@users.noreply.github.com>
Date: Tue, 30 Jan 2024 11:32:58 -0500
Subject: [PATCH] fix: error-display (#8529)

---
 src/js/error-display.js         |  1 -
 src/js/modal-dialog.js          | 82 ++++++++++++++++++---------------
 test/unit/error-display.test.js |  8 ++--
 3 files changed, 48 insertions(+), 43 deletions(-)

diff --git a/src/js/error-display.js b/src/js/error-display.js
index a2ab7d0e3f..20ed994f67 100644
--- a/src/js/error-display.js
+++ b/src/js/error-display.js
@@ -24,7 +24,6 @@ class ErrorDisplay extends ModalDialog {
   constructor(player, options) {
     super(player, options);
     this.on(player, 'error', (e) => {
-      this.close();
       this.open(e);
     });
   }
diff --git a/src/js/modal-dialog.js b/src/js/modal-dialog.js
index d8d181ad06..1c9a3a00fe 100644
--- a/src/js/modal-dialog.js
+++ b/src/js/modal-dialog.js
@@ -99,7 +99,8 @@ class ModalDialog extends Component {
       'aria-describedby': `${this.id()}_description`,
       'aria-hidden': 'true',
       'aria-label': this.label(),
-      'role': 'dialog'
+      'role': 'dialog',
+      'aria-live': 'polite'
     });
   }
 
@@ -156,51 +157,56 @@ class ModalDialog extends Component {
    * @fires ModalDialog#modalopen
    */
   open() {
-    if (!this.opened_) {
-      const player = this.player();
-
-      /**
-        * Fired just before a `ModalDialog` is opened.
-        *
-        * @event ModalDialog#beforemodalopen
-        * @type {Event}
-        */
-      this.trigger('beforemodalopen');
-      this.opened_ = true;
-
-      // Fill content if the modal has never opened before and
-      // never been filled.
-      if (this.options_.fillAlways || !this.hasBeenOpened_ && !this.hasBeenFilled_) {
+    if (this.opened_) {
+      if (this.options_.fillAlways) {
         this.fill();
       }
+      return;
+    }
 
-      // If the player was playing, pause it and take note of its previously
-      // playing state.
-      this.wasPlaying_ = !player.paused();
-
-      if (this.options_.pauseOnOpen && this.wasPlaying_) {
-        player.pause();
-      }
+    const player = this.player();
 
-      this.on('keydown', this.handleKeyDown_);
+    /**
+      * Fired just before a `ModalDialog` is opened.
+      *
+      * @event ModalDialog#beforemodalopen
+      * @type {Event}
+      */
+    this.trigger('beforemodalopen');
+    this.opened_ = true;
 
-      // Hide controls and note if they were enabled.
-      this.hadControls_ = player.controls();
-      player.controls(false);
+    // Fill content if the modal has never opened before and
+    // never been filled.
+    if (this.options_.fillAlways || !this.hasBeenOpened_ && !this.hasBeenFilled_) {
+      this.fill();
+    }
 
-      this.show();
-      this.conditionalFocus_();
-      this.el().setAttribute('aria-hidden', 'false');
+    // If the player was playing, pause it and take note of its previously
+    // playing state.
+    this.wasPlaying_ = !player.paused();
 
-      /**
-        * Fired just after a `ModalDialog` is opened.
-        *
-        * @event ModalDialog#modalopen
-        * @type {Event}
-        */
-      this.trigger('modalopen');
-      this.hasBeenOpened_ = true;
+    if (this.options_.pauseOnOpen && this.wasPlaying_) {
+      player.pause();
     }
+
+    this.on('keydown', this.handleKeyDown_);
+
+    // Hide controls and note if they were enabled.
+    this.hadControls_ = player.controls();
+    player.controls(false);
+
+    this.show();
+    this.conditionalFocus_();
+    this.el().setAttribute('aria-hidden', 'false');
+
+    /**
+      * Fired just after a `ModalDialog` is opened.
+      *
+      * @event ModalDialog#modalopen
+      * @type {Event}
+      */
+    this.trigger('modalopen');
+    this.hasBeenOpened_ = true;
   }
 
   /**
diff --git a/test/unit/error-display.test.js b/test/unit/error-display.test.js
index 3248078545..802483c799 100644
--- a/test/unit/error-display.test.js
+++ b/test/unit/error-display.test.js
@@ -54,10 +54,10 @@ QUnit.test('should update errorDisplay when several errors occur in succession',
     'Error 2',
     'error display contentEl textContent has been updated with the new error message'
   );
-  assert.strictEqual(events.beforemodalopen, 2, 'beforemodalopen has been called for the second time');
-  assert.strictEqual(events.modalopen, 2, 'modalopen has been called for the second time');
-  assert.strictEqual(events.beforemodalclose, 1, 'beforemodalclose was called once');
-  assert.strictEqual(events.modalclose, 1, 'modalclose was called once');
+  assert.strictEqual(events.beforemodalopen, 1, 'beforemodalopen was called once');
+  assert.strictEqual(events.modalopen, 1, 'modalopen has been called once');
+  assert.strictEqual(events.beforemodalclose, 0, 'beforemodalclose was not called');
+  assert.strictEqual(events.modalclose, 0, 'modalclose was not called');
 
   player.dispose();
 });