videojs / videojs-errors

A video.js plugin that displays error messages to video viewers.
Other
87 stars 29 forks source link

initializing with video js options is not working #121

Open axten opened 6 years ago

axten commented 6 years ago

Description

plugin initializing with video js options is not working.

Steps to reproduce

just do:

import videojs from 'video.js';
import 'videojs-errors';

const options = {
    plugins: {
        errors: { 
            errors: {
                FOO: {
                    headline: 'headline',
                    message: 'This is a custom error message'
                }
            }
        }
    }
};

const player = videojs(element, options, () => {

    setTimeout(() => {
        console.log(player.errors.getAll()); // contains FOO error

        player.error({code: 'FOO'}); // empty error screen appears

    },2000);
});

Results

Expected

custom error should be visible

Actual

error screen appears empty (except a white X)

Error output

-

Additional Information

videojs

6.8

errors plugin

4.1

axten commented 6 years ago

I digged deeper and found out that the error content html is build properly. something went wrong after using fillWithmethod of the dialog:

https://github.com/brightcove/videojs-errors/blob/ee400679cde77971117bc0aab1ad50866c010d98/src/plugin.js#L271

seams to be a race condition with: https://github.com/videojs/video.js/blob/2da7af1137e3c8f3b14bd603f17fef3173fe2da0/src/js/modal-dialog.js#L183

so errors plugin fills the dialog and dialog refills it on open with empty content o_0 @gkatsev any ideas how to fix that?

axten commented 6 years ago

https://github.com/brightcove/videojs-errors/blob/ee400679cde77971117bc0aab1ad50866c010d98/src/plugin.js#L240

adding display.options_.fillAlways = false; after that line fixed the race condition. but a bit hacky...

gkatsev commented 6 years ago

this may be that videojs-errors expects the player to be created when it is initialized and so initializing it via options is too early for it. We probably should guard against that.

axten commented 6 years ago

So now I'm sure that it's a race condition between the error event handlers: https://github.com/videojs/video.js/blob/2da7af1137e3c8f3b14bd603f17fef3173fe2da0/src/js/error-display.js#L27 https://github.com/brightcove/videojs-errors/blob/ee400679cde77971117bc0aab1ad50866c010d98/src/plugin.js#L320

They simply change order when errrors-plugin is invoked later by player.errors(). In this case, the error does not occure:

  1. error-display#on('error') calls-> open() calls-> fill() <-- then it is empty
  2. errors-plugin#on('error') calls -> fillWith()

but with options init, these handlers change order and the custom error modal gets overriden.

axten commented 6 years ago

ping @gkatsev

axten commented 5 years ago

any plans to fix this?

jomarquez21 commented 3 years ago

some update?