fix: ensure the default ID of the first player is 'vjs_video_3' as so…

…me people have relied on this (#6216)

When a player is created without an id on the embed code, Video.js automatically assigns it one based on an auto-incrementing number (a.k.a. a GUID). For the longest time, this has happened to result in the default id of the first player being vjs_video_3.

It was never intended for users to rely on this value being consistent, but users do strange and inadvisable things.

PR #6103 had an unintended side effect in that it changed the default id to vjs_video_2, which we worry could affect some users of Video.js.

src/js/setup.js

@@ -5,7 +5,6 @@
  * @module setup
  */
 import * as Dom from './utils/dom';
-import * as Events from './utils/events.js';
 import document from 'global/document';
 import window from 'global/window';
 
@@ -79,21 +78,34 @@ function autoSetupTimeout(wait, vjs) {
   window.setTimeout(autoSetup, wait);
 }
 
-if (Dom.isReal() && document.readyState === 'complete') {
+/**
+ * Used to set the internal tracking of window loaded state to true.
+ *
+ * @private
+ */
+function setWindowLoaded() {
   _windowLoaded = true;
-} else {
-  /**
-   * Listen for the load event on window, and set _windowLoaded to true.
-   *
-   * @listens load
-   */
-  Events.one(window, 'load', function() {
-    _windowLoaded = true;
-  });
+  window.removeEventListener('load', setWindowLoaded);
+}
+
+if (Dom.isReal()) {
+  if (document.readyState === 'complete') {
+    setWindowLoaded();
+  } else {
+    /**
+     * Listen for the load event on window, and set _windowLoaded to true.
+     *
+     * We use a standard event listener here to avoid incrementing the GUID
+     * before any players are created.
+     *
+     * @listens load
+     */
+    window.addEventListener('load', setWindowLoaded);
+  }
 }
 
 /**
- * check if the document has been loaded
+ * check if the window has been loaded
  */
 const hasLoaded = function() {
   return _windowLoaded;

src/js/utils/guid.js

@@ -3,11 +3,20 @@
  * @module guid
  */
 
+// Default value for GUIDs. This allows us to reset the GUID counter in tests.
+//
+// The initial GUID is 3 because some users have come to rely on the first
+// default player ID ending up as `vjs_video_3`.
+//
+// See: https://github.com/videojs/video.js/pull/6216
+const _initialGuid = 3;
+
 /**
  * Unique ID for an element or function
+ *
  * @type {Number}
  */
-let _guid = 1;
+let _guid = _initialGuid;
 
 /**
  * Get a unique auto-incrementing ID by number that has not been returned before.
@@ -18,3 +27,10 @@ let _guid = 1;
 export function newGUID() {
   return _guid++;
 }
+
+/**
+ * Reset the unique auto-incrementing ID for testing only.
+ */
+export function resetGuidInTestsOnly() {
+  _guid = _initialGuid;
+}

test/unit/player.test.js

@@ -14,6 +14,7 @@ import sinon from 'sinon';
 import window from 'global/window';
 import * as middleware from '../../src/js/tech/middleware.js';
 import * as Events from '../../src/js/utils/events.js';
+import * as Guid from '../../src/js/utils/guid.js';
 
 QUnit.module('Player', {
   beforeEach() {
@@ -31,6 +32,17 @@ QUnit.module('Player', {
   }
 });
 
+QUnit.test('the default ID of the first player remains "vjs_video_3"', function(assert) {
+  Guid.resetGuidInTestsOnly();
+  const tag = document.createElement('video');
+
+  tag.className = 'video-js';
+
+  const player = TestHelpers.makePlayer({}, tag);
+
+  assert.strictEqual(player.id(), 'vjs_video_3', 'id is correct');
+});
+
 QUnit.test('should create player instance that inherits from component and dispose it', function(assert) {
   const player = TestHelpers.makePlayer();