fix: don't always use fastSeek when available. (#7527)

We were always setting `scrubbing(true)` on mouse down. This means, that
we'd use `fastSeek` even when seeking while clicking, rather than only
when scrubbing.

The main fix involves knowing in `handleMouseMove` whether we were
called directly as a `mousemove` handler or whether it was called from
`handleMouseDown`. This means that when `handleMouseMove` is called via
`handleMouseDown` we can skip setting `scrubbing(true)` and only do it
when we are scrubbing directly.

src/js/control-bar/progress-control/seek-bar.js

@@ -256,7 +256,6 @@ class SeekBar extends Slider {
 
     // Stop event propagation to prevent double fire in progress-control.js
     event.stopPropagation();
-    this.player_.scrubbing(true);
 
     this.videoWasPlaying = !this.player_.paused();
     this.player_.pause();
@@ -269,13 +268,19 @@ class SeekBar extends Slider {
    *
    * @param {EventTarget~Event} event
    *        The `mousemove` event that caused this to run.
+   * @param {boolean} mouseDown this is a flag that should be set to true if `handleMouseMove` is called directly. It allows us to skip things that should not happen if coming from mouse down but should happen on regular mouse move handler. Defaults to false
    *
    * @listens mousemove
    */
-  handleMouseMove(event) {
+  handleMouseMove(event, mouseDown = false) {
     if (!Dom.isSingleLeftClick(event)) {
       return;
     }
+
+    if (!mouseDown && !this.player_.scrubbing()) {
+      this.player_.scrubbing(true);
+    }
+
     let newTime;
     const distance = this.calculateDistance(event);
     const liveTracker = this.player_.liveTracker;

src/js/slider/slider.js

@@ -180,7 +180,7 @@ class Slider extends Component {
     this.on(doc, 'touchmove', this.handleMouseMove_);
     this.on(doc, 'touchend', this.handleMouseUp_);
 
-    this.handleMouseMove(event);
+    this.handleMouseMove(event, true);
   }
 
   /**
@@ -192,6 +192,7 @@ class Slider extends Component {
    * @param {EventTarget~Event} event
    *        `mousedown`, `mousemove`, `touchstart`, or `touchmove` event that triggered
    *        this function
+   * @param {boolean} mouseDown this is a flag that should be set to true if `handleMouseMove` is called directly. It allows us to skip things that should not happen if coming from mouse down but should happen on regular mouse move handler. Defaults to false.
    *
    * @listens mousemove
    * @listens touchmove

test/unit/controls.test.js

@@ -1,4 +1,5 @@
 /* eslint-env qunit */
+import EventTarget from '../../src/js/event-target.js';
 import VolumeControl from '../../src/js/control-bar/volume-control/volume-control.js';
 import MuteToggle from '../../src/js/control-bar/mute-toggle.js';
 import VolumeBar from '../../src/js/control-bar/volume-control/volume-bar.js';
@@ -8,6 +9,7 @@ import Slider from '../../src/js/slider/slider.js';
 import PictureInPictureToggle from '../../src/js/control-bar/picture-in-picture-toggle.js';
 import FullscreenToggle from '../../src/js/control-bar/fullscreen-toggle.js';
 import ControlBar from '../../src/js/control-bar/control-bar.js';
+import SeekBar from '../../src/js/control-bar/progress-control/seek-bar.js';
 import TestHelpers from './test-helpers.js';
 import document from 'global/document';
 import sinon from 'sinon';
@@ -141,6 +143,35 @@ QUnit.test('calculateDistance should use changedTouches, if available', function
   slider.dispose();
 });
 
+QUnit.test("SeekBar doesn't set scrubbing on mouse down, only on mouse move", function(assert) {
+  const player = TestHelpers.makePlayer();
+  const scrubbingSpy = sinon.spy(player, 'scrubbing');
+  const seekBar = new SeekBar(player);
+  const doc = new EventTarget();
+
+  // mousemove is listened to on the document.
+  // Specifically, we check the ownerDocument of the seekBar's bar.
+  // Therefore, we want to mock it out to be able to trigger mousemove
+  seekBar.bar.dispose();
+  seekBar.bar.el_ = new EventTarget();
+  seekBar.bar.el_.ownerDocument = doc;
+
+  seekBar.trigger('mousedown');
+  assert.ok(scrubbingSpy.calledWith(), 'called scrubbing as a getter');
+  assert.notOk(scrubbingSpy.calledWith(true), 'did not set scrubbing true');
+
+  player.scrubbing(false);
+
+  scrubbingSpy.resetHistory();
+
+  doc.trigger('mousemove');
+  assert.ok(scrubbingSpy.calledWith(), 'called scrubbing as a getter');
+  assert.ok(scrubbingSpy.calledWith(true), 'did set scrubbing true');
+
+  seekBar.dispose();
+  player.dispose();
+});
+
 QUnit.test('playback rate button is hidden by default', function(assert) {
   assert.expect(1);