diff --git a/lib/widgets/video_controls/helpers/eager_horizontal_drag_recognizer.dart b/lib/widgets/video_controls/helpers/eager_horizontal_drag_recognizer.dart new file mode 100644 index 00000000..1cd7a5da --- /dev/null +++ b/lib/widgets/video_controls/helpers/eager_horizontal_drag_recognizer.dart @@ -0,0 +1,33 @@ +import 'package:flutter/gestures.dart'; + +/// A [HorizontalDragGestureRecognizer] that claims the gesture arena the +/// moment a pointer lands on it, instead of waiting for horizontal movement +/// to exceed the touch slop. +/// +/// Scrubbers must own any touch that starts on them (standard video-player +/// behavior). With slop-based recognition, competing recognizers higher in +/// the tree — the content-strip vertical drag or the long-press 2x-speed +/// handler — can win the arena and silently eat the gesture, making the +/// timeline appear to "stick" (#1302). +/// +/// Tracks a single pointer: additional fingers placed on the scrubber +/// mid-drag are ignored rather than averaged into the drag. +class EagerHorizontalDragGestureRecognizer extends HorizontalDragGestureRecognizer { + EagerHorizontalDragGestureRecognizer({super.debugOwner}); + + int? _activePointer; + + @override + void addAllowedPointer(PointerDownEvent event) { + if (_activePointer != null) return; + _activePointer = event.pointer; + super.addAllowedPointer(event); + resolve(GestureDisposition.accepted); + } + + @override + void didStopTrackingLastPointer(int pointer) { + super.didStopTrackingLastPointer(pointer); + _activePointer = null; + } +} diff --git a/lib/widgets/video_controls/parts/playback_input.dart b/lib/widgets/video_controls/parts/playback_input.dart index 4d933ca2..6279fae9 100644 --- a/lib/widgets/video_controls/parts/playback_input.dart +++ b/lib/widgets/video_controls/parts/playback_input.dart @@ -89,6 +89,9 @@ extension _PlexVideoControlsPlaybackInputMethods on _PlexVideoControlsState { /// Throttled seek for timeline slider - executes immediately then throttles to 200ms void _throttledSeek(Duration position) { + // Hold before the transcoding early-return so a slow scrub never loses + // the controls to auto-hide mid-drag (idempotent while held). + widget.chromeController.hold(PlayerChromeHold.scrub); if (widget.isTranscoding) return; _seekThrottle([position]); } @@ -97,6 +100,7 @@ extension _PlexVideoControlsPlaybackInputMethods on _PlexVideoControlsState { void _finalizeSeek(Duration position) { _seekThrottle.cancel(); unawaited(_seekToPosition(position)); + widget.chromeController.release(PlayerChromeHold.scrub); } bool get _isTouchTapSuppressed { diff --git a/lib/widgets/video_controls/player_chrome_controller.dart b/lib/widgets/video_controls/player_chrome_controller.dart index b3590f50..4703c2a3 100644 --- a/lib/widgets/video_controls/player_chrome_controller.dart +++ b/lib/widgets/video_controls/player_chrome_controller.dart @@ -5,7 +5,7 @@ import 'package:flutter/material.dart' show BuildContext, ListenableBuilder, MouseRegion, StatelessWidget, SystemMouseCursors, Widget; /// Reasons that keep the video-player chrome visible and suppress auto-hide. -enum PlayerChromeHold { pip, contentStrip, promptInteraction } +enum PlayerChromeHold { pip, contentStrip, promptInteraction, scrub } /// Focus target to request after chrome has rebuilt visible controls. enum PlayerChromeFocusTarget { playPause, timeline } diff --git a/lib/widgets/video_controls/video_controls.dart b/lib/widgets/video_controls/video_controls.dart index bdef0406..4f8ee014 100644 --- a/lib/widgets/video_controls/video_controls.dart +++ b/lib/widgets/video_controls/video_controls.dart @@ -541,6 +541,8 @@ class _PlexVideoControlsState extends State _skipButtonDismissTimer?.cancel(); _singleTapTimer?.cancel(); _seekThrottle.cancel(); + // A player exit mid-scrub must not leak the hold into the route teardown. + widget.chromeController.release(PlayerChromeHold.scrub, notify: false, restartAutoHide: false); _playingSubscription?.cancel(); _completedSubscription?.cancel(); _positionSubscription?.cancel(); diff --git a/lib/widgets/video_controls/widgets/live_timeline_bar.dart b/lib/widgets/video_controls/widgets/live_timeline_bar.dart index 5f82e226..a7aae5f5 100644 --- a/lib/widgets/video_controls/widgets/live_timeline_bar.dart +++ b/lib/widgets/video_controls/widgets/live_timeline_bar.dart @@ -1,3 +1,4 @@ +import 'package:flutter/gestures.dart' show DragStartBehavior; import 'package:flutter/material.dart'; import '../../../models/livetv_capture_buffer.dart'; @@ -5,6 +6,7 @@ import '../../../mpv/mpv.dart'; import '../../../focus/focusable_wrapper.dart'; import '../../../utils/formatters.dart'; import '../../clickable_cursor.dart'; +import '../helpers/eager_horizontal_drag_recognizer.dart'; /// Timeline bar for live TV time-shift. /// @@ -137,11 +139,28 @@ class _LiveTimelineBarState extends State { builder: (context) { return ClickableCursor( enabled: widget.enabled, - child: GestureDetector( - onHorizontalDragStart: widget.enabled ? (_) => _onDragStart() : null, - onHorizontalDragUpdate: widget.enabled ? (details) => _onDragUpdate(details, _widthOf(context)) : null, - onHorizontalDragEnd: widget.enabled ? (_) => _onDragEnd() : null, - onTapUp: widget.enabled ? (details) => _onTap(details, _widthOf(context)) : null, + // Eager claim: a touch that lands on the scrubber belongs to it + // from pointer-down, so ancestor recognizers can't steal the drag + // (#1302). A plain tap is onStart+onEnd, which seeks to the + // tapped position. + child: RawGestureDetector( + behavior: HitTestBehavior.opaque, + gestures: widget.enabled + ? { + EagerHorizontalDragGestureRecognizer: + GestureRecognizerFactoryWithHandlers( + () => + EagerHorizontalDragGestureRecognizer(debugOwner: this) + ..dragStartBehavior = DragStartBehavior.down, + (instance) { + instance.onStart = (details) => _onDragStart(details, _widthOf(context)); + instance.onUpdate = (details) => _onDragUpdate(details, _widthOf(context)); + instance.onEnd = (_) => _onDragEnd(); + instance.onCancel = _onDragEnd; + }, + ), + } + : const {}, child: SizedBox( width: double.infinity, height: 24, @@ -154,33 +173,34 @@ class _LiveTimelineBarState extends State { ); } - void _onDragStart() { + void _onDragStart(DragStartDetails details, double width) { setState(() { _isDragging = true; _dragPositionEpoch = _currentEpoch(widget.player.state.position); }); + _applyDrag(details.localPosition.dx, width); } void _onDragUpdate(DragUpdateDetails details, double width) { + if (!_isDragging) return; + _applyDrag(details.localPosition.dx, width); + } + + void _applyDrag(double dx, double width) { if (width <= 0) return; - final fraction = (details.localPosition.dx / width).clamp(0.0, 1.0); + final fraction = (dx / width).clamp(0.0, 1.0); setState(() { _dragPositionEpoch = _fractionToEpoch(fraction); }); } + /// Shared by onEnd and onCancel so an interrupted drag still finalizes. void _onDragEnd() { + if (!_isDragging) return; final target = _dragPositionEpoch; setState(() => _isDragging = false); widget.onSeekEnd?.call(target); } - - void _onTap(TapUpDetails details, double width) { - if (width <= 0) return; - final fraction = (details.localPosition.dx / width).clamp(0.0, 1.0); - final target = _fractionToEpoch(fraction); - widget.onSeekEnd?.call(target); - } } class _LiveTimelinePainter extends CustomPainter { diff --git a/lib/widgets/video_controls/widgets/timeline_slider.dart b/lib/widgets/video_controls/widgets/timeline_slider.dart index 9ddc23a8..0c9d7a8b 100644 --- a/lib/widgets/video_controls/widgets/timeline_slider.dart +++ b/lib/widgets/video_controls/widgets/timeline_slider.dart @@ -1,3 +1,4 @@ +import 'package:flutter/gestures.dart' show DragStartBehavior; import 'package:flutter/material.dart'; import '../../../media/media_source_info.dart'; import '../../../mpv/models.dart'; @@ -6,6 +7,7 @@ import '../../../focus/focusable_wrapper.dart'; import '../../../focus/input_mode_tracker.dart'; import '../../../services/scrub_preview_source.dart'; import '../../../utils/formatters.dart'; +import '../helpers/eager_horizontal_drag_recognizer.dart'; import '../painters/buffer_range_painter.dart'; /// Timeline slider with chapter markers for video playback @@ -75,12 +77,70 @@ class _TimelineSliderState extends State { ScrubFrame? _hoverFrame; Object? _hoverFrameKey; bool _isFocused = false; + bool _scrubbing = false; // Must match the slider track inset: max(overlayRadius, thumbRadius) static const _sliderPadding = 0.0; static const _thumbWidth = 160.0; + @override + void didUpdateWidget(TimelineSlider oldWidget) { + super.didUpdateWidget(oldWidget); + if (oldWidget.enabled && !widget.enabled && _scrubbing) { + // The gestures map is only registered while enabled, so the swap + // disposes the recognizer without firing onCancel; finalize after this + // frame so the parent isn't notified mid-build. + WidgetsBinding.instance.addPostFrameCallback((_) { + if (mounted) _handleScrubEnd(); + }); + } + } + + void _handleScrubStart(DragStartDetails details, BuildContext sliderContext) { + if (widget.duration.inMilliseconds <= 0) return; + _scrubbing = true; + _applyScrub(details.localPosition.dx, sliderContext); + } + + void _handleScrubUpdate(DragUpdateDetails details, BuildContext sliderContext) { + if (!_scrubbing) return; + _applyScrub(details.localPosition.dx, sliderContext); + } + + void _applyScrub(double dx, BuildContext sliderContext) { + final durationMs = widget.duration.inMilliseconds; + final trackWidth = _sliderWidthOf(sliderContext) - 2 * _sliderPadding; + if (durationMs <= 0 || trackWidth <= 0) return; + final fraction = ((dx - _sliderPadding) / trackWidth).clamp(0.0, 1.0); + final value = fraction * durationMs; + setState(() => _dragValue = value); + widget.onSeek(Duration(milliseconds: value.round())); + } + + /// Shared by onEnd and onCancel: a cancelled scrub still finalizes at the + /// last position (Material Slider parity) so `_dragValue` is never stuck. + void _handleScrubEnd() { + if (!_scrubbing) return; + _scrubbing = false; + final value = _dragValue; + setState(() => _dragValue = null); + if (value != null) widget.onSeekEnd(Duration(milliseconds: value.round())); + } + + /// Discrete a11y step (VoiceOver/TalkBack swipe): a complete seek. + void _semanticSeekBy(Duration delta) { + final durationMs = widget.duration.inMilliseconds; + if (durationMs <= 0) return; + final base = _dragValue ?? widget.position.inMilliseconds.toDouble(); + final target = (base + delta.inMilliseconds).clamp(0.0, durationMs.toDouble()); + widget.onSeekEnd(Duration(milliseconds: target.round())); + } + + // Keeps the visual Slider in its enabled style; real input goes through the + // eager scrub recognizer above it. + static void _noopSliderChanged(double _) {} + Object? _scrubFrameKey(ScrubFrame? frame) { return switch (frame) { null => null, @@ -230,74 +290,108 @@ class _TimelineSliderState extends State { _mousePosition != null || (widget.showKeyRepeatThumbnail && widget.thumbnailDataBuilder != null)); - Widget buildSlider(Widget? tooltip) { - return Stack( - clipBehavior: Clip.none, - alignment: .center, - children: [ - // Buffer range + segmented background track (with chapter gaps) - Positioned.fill( - child: IgnorePointer( - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: _sliderPadding), - child: CustomPaint( - painter: BufferRangePainter( - ranges: widget.bufferRanges, - duration: widget.duration, - chapters: widget.chaptersLoaded && widget.showChapterMarkersOnTimeline ? widget.chapters : const [], + // The element tree below is structurally identical on every build: an + // in-flight drag must never be disposed mid-gesture by a tree flip, and + // the eager recognizer claims the arena at pointer-down so ancestor + // recognizers (content-strip swipe, long-press 2x) can't steal a scrub + // (#1302). The Material Slider is visual-only. + Widget slider = Builder( + builder: (sliderContext) => RawGestureDetector( + behavior: HitTestBehavior.opaque, + excludeFromSemantics: true, + gestures: widget.enabled + ? { + EagerHorizontalDragGestureRecognizer: + GestureRecognizerFactoryWithHandlers( + () => + EagerHorizontalDragGestureRecognizer(debugOwner: this) + ..dragStartBehavior = DragStartBehavior.down, + (instance) { + instance.onStart = (details) => _handleScrubStart(details, sliderContext); + instance.onUpdate = (details) => _handleScrubUpdate(details, sliderContext); + instance.onEnd = (_) => _handleScrubEnd(); + instance.onCancel = _handleScrubEnd; + }, + ), + } + : const {}, + child: Stack( + clipBehavior: Clip.none, + alignment: .center, + children: [ + // Buffer range + segmented background track (with chapter gaps) + Positioned.fill( + child: IgnorePointer( + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: _sliderPadding), + child: CustomPaint( + painter: BufferRangePainter( + ranges: widget.bufferRanges, + duration: widget.duration, + chapters: widget.chaptersLoaded && widget.showChapterMarkersOnTimeline + ? widget.chapters + : const [], + ), ), ), ), ), - ), - // Slider - use IgnorePointer to block interaction while preserving visual style - IgnorePointer( - ignoring: !widget.enabled, - child: SliderTheme( - data: SliderTheme.of(context).copyWith( - trackHeight: 8, - trackGap: 0, - padding: .zero, - overlayShape: const RoundSliderOverlayShape(overlayRadius: 0), - tickMarkShape: SliderTickMarkShape.noTickMark, - thumbSize: WidgetStatePropertyAll( - (!InputModeTracker.isKeyboardMode(context) || _isFocused) ? const Size(4, 20) : Size.zero, - ), + Semantics( + label: t.videoControls.timelineSlider, + slider: true, + value: formatDurationTimestamp(displayPosition), + increasedValue: formatDurationTimestamp( + Duration(milliseconds: (displayValue + 10000).clamp(0.0, max).round()), ), - child: Semantics( - label: t.videoControls.timelineSlider, - slider: true, - child: Slider( - value: displayValue, - min: 0.0, - max: max, - onChanged: (value) { - setState(() => _dragValue = value); - widget.onSeek(Duration(milliseconds: value.toInt())); - }, - onChangeEnd: (value) { - setState(() => _dragValue = null); - widget.onSeekEnd(Duration(milliseconds: value.toInt())); - }, - activeColor: Colors.white, - inactiveColor: Colors.transparent, + decreasedValue: formatDurationTimestamp( + Duration(milliseconds: (displayValue - 10000).clamp(0.0, max).round()), + ), + enabled: widget.enabled, + onIncrease: widget.enabled && durationMs > 0 ? () => _semanticSeekBy(const Duration(seconds: 10)) : null, + onDecrease: widget.enabled && durationMs > 0 ? () => _semanticSeekBy(const Duration(seconds: -10)) : null, + child: ExcludeSemantics( + child: IgnorePointer( + child: SliderTheme( + data: SliderTheme.of(context).copyWith( + trackHeight: 8, + trackGap: 0, + padding: .zero, + overlayShape: const RoundSliderOverlayShape(overlayRadius: 0), + tickMarkShape: SliderTickMarkShape.noTickMark, + thumbSize: WidgetStatePropertyAll( + (!InputModeTracker.isKeyboardMode(context) || _isFocused) ? const Size(4, 20) : Size.zero, + ), + ), + child: Slider( + value: displayValue, + min: 0.0, + max: max, + onChanged: _noopSliderChanged, + activeColor: Colors.white, + inactiveColor: Colors.transparent, + ), + ), ), ), ), - ), - ?tooltip, - ], - ); - } - - Widget slider = hasTooltip - ? LayoutBuilder( - builder: (context, constraints) { - final tooltip = _buildActiveTooltip(constraints.maxWidth, durationMs, displayValue, displayPosition); - return buildSlider(tooltip); - }, - ) - : buildSlider(null); + // Tooltip layer: a permanent child so showing/hiding the tooltip + // never changes the structure around the slider. + Positioned.fill( + child: IgnorePointer( + child: LayoutBuilder( + builder: (context, constraints) { + final tooltip = hasTooltip + ? _buildActiveTooltip(constraints.maxWidth, durationMs, displayValue, displayPosition) + : null; + return Stack(clipBehavior: Clip.none, children: [?tooltip]); + }, + ), + ), + ), + ], + ), + ), + ); // Wrap with FocusableWrapper when focusNode is provided if (widget.focusNode != null) { diff --git a/test/widgets/video_controls_test.dart b/test/widgets/video_controls_test.dart index af8e89c9..59af229b 100644 --- a/test/widgets/video_controls_test.dart +++ b/test/widgets/video_controls_test.dart @@ -418,6 +418,167 @@ void main() { expect(slider.value, 0.0); expect(slider.max, 0.0); }); + + Future pumpScrubSlider( + WidgetTester tester, { + required List seeks, + required List seekEnds, + Duration duration = const Duration(minutes: 10), + bool enabled = true, + Widget Function(Widget child)? wrap, + }) async { + Widget slider = SizedBox( + width: 400, + child: TimelineSlider( + position: const Duration(minutes: 1), + duration: duration, + chapters: const [], + chaptersLoaded: true, + enabled: enabled, + onSeek: seeks.add, + onSeekEnd: seekEnds.add, + ), + ); + if (wrap != null) slider = wrap(slider); + await tester.pumpWidget( + MaterialApp( + home: Scaffold(body: Center(child: slider)), + ), + ); + } + + testWidgets('touch drag survives tooltip appearance and finalizes once', (tester) async { + final seeks = []; + final seekEnds = []; + await pumpScrubSlider(tester, seeks: seeks, seekEnds: seekEnds); + + // Down at the center (200/400 → 5min), drag +100px (→ 7.5min). The + // first scrub event makes the tooltip appear; the drag must keep + // tracking through that rebuild and finalize exactly once. + final gesture = await tester.startGesture(tester.getCenter(find.byType(TimelineSlider))); + await tester.pump(); + await gesture.moveBy(const Offset(50, 0)); + await tester.pump(); + await gesture.moveBy(const Offset(50, 0)); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + expect(seeks, isNotEmpty); + expect(seekEnds, hasLength(1)); + expect(seekEnds.single.inMilliseconds, closeTo(const Duration(minutes: 7, seconds: 30).inMilliseconds, 2000)); + }); + + testWidgets('tap seeks to the tapped position', (tester) async { + final seeks = []; + final seekEnds = []; + await pumpScrubSlider(tester, seeks: seeks, seekEnds: seekEnds); + + final topLeft = tester.getTopLeft(find.byType(TimelineSlider)); + final size = tester.getSize(find.byType(TimelineSlider)); + final gesture = await tester.startGesture(Offset(topLeft.dx + size.width * 0.75, topLeft.dy + size.height / 2)); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + expect(seekEnds, hasLength(1)); + expect(seekEnds.single.inMilliseconds, closeTo(const Duration(minutes: 7, seconds: 30).inMilliseconds, 2000)); + }); + + testWidgets('drag starting on the slider is never stolen by ancestor recognizers', (tester) async { + final seeks = []; + final seekEnds = []; + var verticalDragUpdates = 0; + var longPresses = 0; + await pumpScrubSlider( + tester, + seeks: seeks, + seekEnds: seekEnds, + wrap: (child) => GestureDetector( + behavior: HitTestBehavior.translucent, + onVerticalDragUpdate: (_) => verticalDragUpdates++, + onLongPressStart: (_) => longPresses++, + child: child, + ), + ); + + // Press-aim-drag: hold past the long-press deadline, then drag with a + // vertical-dominant start. Without the eager claim, the long-press or + // the vertical recognizer wins and the scrub is eaten. + final gesture = await tester.startGesture(tester.getCenter(find.byType(TimelineSlider))); + await tester.pump(const Duration(milliseconds: 600)); + for (var i = 0; i < 4; i++) { + await gesture.moveBy(const Offset(8, 12)); + await tester.pump(); + } + await gesture.moveBy(const Offset(50, 0)); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + expect(seekEnds, hasLength(1)); + expect(verticalDragUpdates, 0); + expect(longPresses, 0); + }); + + testWidgets('ignores input when disabled', (tester) async { + final seeks = []; + final seekEnds = []; + await pumpScrubSlider(tester, seeks: seeks, seekEnds: seekEnds, enabled: false); + + final gesture = await tester.startGesture(tester.getCenter(find.byType(TimelineSlider))); + await tester.pump(); + await gesture.moveBy(const Offset(50, 0)); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + expect(seeks, isEmpty); + expect(seekEnds, isEmpty); + }); + + testWidgets('ignores input when duration is unknown', (tester) async { + final seeks = []; + final seekEnds = []; + await pumpScrubSlider(tester, seeks: seeks, seekEnds: seekEnds, duration: Duration.zero); + + final gesture = await tester.startGesture(tester.getCenter(find.byType(TimelineSlider))); + await tester.pump(); + await gesture.moveBy(const Offset(50, 0)); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + expect(seeks, isEmpty); + expect(seekEnds, isEmpty); + }); + + testWidgets('second finger is ignored mid-drag', (tester) async { + final seeks = []; + final seekEnds = []; + await pumpScrubSlider(tester, seeks: seeks, seekEnds: seekEnds); + + final center = tester.getCenter(find.byType(TimelineSlider)); + final first = await tester.startGesture(center); + await tester.pump(); + final seeksAfterDown = seeks.length; + + final second = await tester.startGesture(center + const Offset(100, 0)); + await tester.pump(); + await second.moveBy(const Offset(-80, 0)); + await tester.pump(); + expect(seeks.length, seeksAfterDown, reason: 'second pointer must not drive the scrub'); + + await first.moveBy(const Offset(40, 0)); + await tester.pump(); + await first.up(); + await second.up(); + await tester.pump(); + + // 240/400 of 10min → 6min: follows the first pointer only. + expect(seekEnds, hasLength(1)); + expect(seekEnds.single.inMilliseconds, closeTo(const Duration(minutes: 6).inMilliseconds, 2000)); + }); }); group('SyncOffsetControl', () {