Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Nov 17, 2021
Merged

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

merged 5 commits into from
Nov 17, 2021

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Nov 15, 2021

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.

Fixes #6988, fixes #7360

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.
@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #7527 (c2d1de6) into main (6c67c30) will increase coverage by 0.41%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7527      +/-   ##
==========================================
+ Coverage   79.84%   80.26%   +0.41%     
==========================================
  Files         116      116              
  Lines        7309     7321      +12     
  Branches     1764     1769       +5     
==========================================
+ Hits         5836     5876      +40     
+ Misses       1473     1445      -28     
Impacted Files Coverage Δ
src/js/control-bar/progress-control/seek-bar.js 55.19% <100.00%> (+10.13%) ⬆️
src/js/slider/slider.js 59.29% <100.00%> (+11.50%) ⬆️
...-bar/audio-track-controls/audio-track-menu-item.js 3.03% <0.00%> (-0.82%) ⬇️
src/js/tech/html5.js 63.49% <0.00%> (-0.12%) ⬇️
src/js/player.js 87.94% <0.00%> (+0.20%) ⬆️
src/js/utils/dom.js 64.34% <0.00%> (+1.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c67c30...c2d1de6. Read the comment docs.

src/js/slider/slider.js Outdated Show resolved Hide resolved
test/unit/controls.test.js Outdated Show resolved Hide resolved
@gkatsev gkatsev merged commit df927de into main Nov 17, 2021
@gkatsev gkatsev deleted the fast-seek-fix branch November 17, 2021 20:59
@gkatsev gkatsev removed the needs: LGTM Needs one or more additional approvals label Dec 9, 2021
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dash fastSeek issue in safari Seeking via tab on seekBar does not work properly on iOS
2 participants