fix: error-display

[harisha-swaminathan]

Description

This recent change closes the error modal and opens it again inorder to ensure that the error display component is updated with the new error message when consecutive errors occur.

But when this is used with the videojs-errors plugin, no error is displayed after the second error occurs. This is because when the error display is closed after the second error, error is cleared and set to null here.

Specific Changes proposed

The suggested change is to just update the modal's content with the latest error message instead of closing and opening the error display.

Requirements Checklist

• Feature implemented / Bug fixed
• If necessary, more likely in a feature request than a bug fix
  • Change has been verified in an actual browser (Chrome, Firefox, IE)
  • Unit Tests updated or fixed
  • Docs/guides updated
  • Example created (starter template on JSBin)
• Reviewed by Two Core Contributors

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d2dc5b9) 82.65% compared to head (864da99) 82.50%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8529      +/-   ##
==========================================
- Coverage   82.65%   82.50%   -0.15%     
==========================================
  Files         113      113              
  Lines        7605     7661      +56     
  Branches     1828     1848      +20     
==========================================
+ Hits         6286     6321      +35     
- Misses       1319     1340      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[amtins]

@harisha-swaminathan visually, this modification has the same effect. However, I have a question about accessibility. Shouldn't there be a narrated feedback after each error?

Before this modification, if the player had focus and several errors occurred, there was a narrated feedback. Even if the feedback wasn't very descriptive, the user was informed that something had happened.

This case can be tested as follows:

1. open Chrome
2. install the screen reader extension
3. open a page with the player
4. open the developer console
5. paste the script

setTimeout(() => {
    player.src('https://media-url.com/404.m3u8')

    setTimeout(() => {
        player.src('https://media-url.com/not-compatible.rtmp')
    }, 15_000);
}, 10_000);

6. return to the page with the player and click play then pause
7. wait for errors to occur and listen to narrated feedback

It is possible to provide a little more context on the error by modifying the script as follows:

videojs.dom.$('.vjs-error-display .vjs-modal-dialog-content').id = 'vjs-modal-content-id';

videojs.dom.$('.vjs-error-display').setAttribute('aria-describedby', `${videojs.dom.$('.vjs-error-display').getAttribute('aria-describedby')}, vjs-modal-content-id`);

setTimeout(() => {
    player.src('https://media-url.com/404.m3u8')

    setTimeout(() => {
        player.src('https://media-url.com/not-compatible.rtmp')
    }, 15_000);
}, 10_000);

[harisha-swaminathan]

Thank you @amtins

This, somehow, isn't a problem when voiceOver accessibility is used on Mac. However, I'm able to reproduce it with a screen reader extension.

Do you think "aria-live: polite" can be user here instead? Added that in the latest commit and it seems to work as expected.

[roman-bc-dev]

• This change seems to be inline with the existing visible UX. We may want to consider a solution in the future that would allow the user to see both errors in the player modal and not just the last one to be generated.
• Because we can't guarantee the timing between errors, it's quite possible that aria-live: polite here could result in both errors being announced by a screen reader but I think that might actually be preferable to only hearing about the latest error emitted.