Add froogaloop library as a dependency to load-player module

[ntoombs19]

Description

On line 336 of load-player.js the object window.$f is used

this._player = window.$f(this.element.children(':first')[0]);

This object is defined in the froogaloop library. The load-player module is the only place where this object is used. The froogaloop library is dependency for the load-player module but it is not declared as a dependency. Under certain network conditions this could lead to the window.$f object being undefined.

Manual testing scenarios

1. To simulate a network condition that would cause the froogaloop library to load after line 336 is run, make the following change in Magento/ProductVideo/view/frontend/web/js/fotorama-add-video-events.js:

_checkForVimeo: function () {
    var allVideoData = this.options.videoData,
        videoItem;

    for (videoItem in allVideoData) {
        if (allVideoData[videoItem].provider === this.VI) {
            setTimeout(function () {                                           // Added line
                this._loadVimeoJSFramework();
            }, 30000);                                                                  // Added line
        }
    }
},

2. Now add a vimeo video to a product and load that product on the frontend. When the video loads you will see a console message saying window.$f is undefined.

Contribution checklist

[x] Pull request has a meaningful description of its purpose
[x] All commits are accompanied by meaningful commit messages
N/A All new or changed code is covered with unit/integration tests (if applicable)
N/A All automated tests passed successfully (all builds on Travis CI are green)

[omiroshnichenko]

Hi, @ntoombs19. Your solution provide duplicate of 3rd party dependency. Vimeo JS API already loading in fotorama-add-video-events, but in your scenario we have a problem with asynchronous loading of froogaloop lib. I think will be better to add onload handler for existing script loading process and toggle some property that will indicate that script is loaded. Also, it would be nice to add call of callback function, that provide possibility to check in _clickHandler is lib already loaded or we must postpone loadPlayer initialization to callback that will be called after script loading. Here code examples how to do it:

 $.widget('mage.AddFotoramaVideoEvents', {
...
    //Declaring properties
    vimeoJSFrameworkLoaded: false,
    onVimeoJSFramework: function () {},
...
    _loadVimeoJSFramework: function () {
        var element = document.createElement('script'),
              scriptTag = document.getElementsByTagName('script')[0];

        element.async = true;
        element.src = 'https://secure-a.vimeocdn.com/js/froogaloop2.min.js';

        /**
         * Vimeo js framework on load callback.
        */
        element.onload = function () {
            this.onVimeoJSFramework(); //execute callback
            this.vimeoJSFrameworkLoaded = true; //toggle property
        }.bind(this);
        scriptTag.parentNode.insertBefore(element, scriptTag);
    },
...
/**
     *
     * @param {Event} event
     * @private
     */
    _clickHandler: function (event) {
        if ($(event.target).hasClass(this.VU) && $(event.target).find('iframe').length === 0) {

            $(event.target).removeClass(this.VU);

           //Check if lib has been loaded
            if (this.vimeoJSFrameworkLoaded) {
                $(event.target).find('.' + this.PV).productVideoLoader();
            } else {
           //Initialize video loader when lib will be loaded
                this.onVimeoJSFramework = function () {
                    $(event.target).find('.' + this.PV).productVideoLoader();
                }.bind(this);
            }

            $('.' + this.FTAR).addClass(this.isFullscreen ? 'fotorama__arr--shown' : 'fotorama__arr--hidden');
        }
    }
...
}

[ishakhsuvarov]

Hi @ntoombs19
Do you wish to continue working on this PR?
Thanks!

[ntoombs19]

I think this is a necessary change but I don't have the time to implement the change at this time.

[ishakhsuvarov]

@ntoombs19 I will close this PR for now. Please reopen if you would like to continue at any time.
Thank you for collaboration.

[ishakhsuvarov]

@omiroshnichenko Is going to continue on this one.