From 8719166dbfb0621ac8bb2ba5e595ffa13f29672d Mon Sep 17 00:00:00 2001 From: HouKangle Date: Fri, 30 Oct 2026 18:02:02 +0800 Subject: [PATCH] fix(web): prevent browser context menu over task menus (#1627) --- .github/workflows/test.yml | 26 ++ .../widgets/task_context_menu.dart | 4 +- .../widgets/browser_context_menu_region.dart | 81 +++++++ .../browser_context_menu_region_stub.dart | 5 + .../browser_context_menu_region_web.dart | 51 ++++ .../widgets/task_context_menu_test.dart | 223 ++++++++++++++++++ .../test/support/context_menu_event.dart | 1 + .../test/support/context_menu_event_stub.dart | 5 + .../test/support/context_menu_event_web.dart | 25 ++ 9 files changed, 420 insertions(+), 1 deletion(-) create mode 100644 ui/flutter/lib/shared/widgets/browser_context_menu_region.dart create mode 100644 ui/flutter/lib/shared/widgets/browser_context_menu_region_stub.dart create mode 100644 ui/flutter/lib/shared/widgets/browser_context_menu_region_web.dart create mode 100644 ui/flutter/test/features/tasks/presentation/widgets/task_context_menu_test.dart create mode 100644 ui/flutter/test/support/context_menu_event.dart create mode 100644 ui/flutter/test/support/context_menu_event_stub.dart create mode 100644 ui/flutter/test/support/context_menu_event_web.dart diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 80d35a74..467044a6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -86,6 +86,32 @@ jobs: flutter pub get flutter gen-l10n flutter test test/api/downloader_config_test.dart + test-flutter-task-context-menu: + name: Test task context menus and text inputs + runs-on: ubuntu-latest + timeout-minutes: 20 + env: + CHROME_EXECUTABLE: /usr/bin/google-chrome + steps: + - uses: actions/checkout@v3 + - uses: subosito/flutter-action@v2 + with: + flutter-version: ${{ env.FLUTTER_VERSION }} + - name: Prepare Flutter tests + working-directory: ui/flutter + run: | + flutter pub get + flutter gen-l10n + - name: Test task menu actions and native input policy + working-directory: ui/flutter + run: | + flutter test --no-pub test/features/tasks/presentation/widgets/task_context_menu_test.dart test/shared/widgets/app_text_field_test.dart + flutter test --no-pub test/widget_test.dart --plain-name 'task context menu exposes' + - name: Test browser menu cancellation and input policy + working-directory: ui/flutter + run: | + "$CHROME_EXECUTABLE" --version + flutter test --no-pub --platform chrome test/features/tasks/presentation/widgets/task_context_menu_test.dart test/shared/widgets/app_text_field_test.dart test-webview-integration: name: Test webview integration (${{ matrix.name }}) strategy: diff --git a/ui/flutter/lib/features/tasks/presentation/widgets/task_context_menu.dart b/ui/flutter/lib/features/tasks/presentation/widgets/task_context_menu.dart index bf9fb2fb..05216d7c 100644 --- a/ui/flutter/lib/features/tasks/presentation/widgets/task_context_menu.dart +++ b/ui/flutter/lib/features/tasks/presentation/widgets/task_context_menu.dart @@ -5,6 +5,7 @@ import 'package:shadcn_flutter/shadcn_flutter.dart' as shad; import '../../domain/task_record.dart'; import '../../../../l10n/l10n.dart'; +import '../../../../shared/widgets/browser_context_menu_region.dart'; import '../../../../util/util.dart'; class TaskContextActions { @@ -50,7 +51,7 @@ class TaskContextMenu extends StatelessWidget { @override Widget build(BuildContext context) { - return shad.ContextMenu( + final menu = shad.ContextMenu( items: [ shad.MenuButton( key: const ValueKey('task-context-select-all'), @@ -134,6 +135,7 @@ class TaskContextMenu extends StatelessWidget { ], child: child, ); + return BrowserContextMenuRegion(child: menu); } } diff --git a/ui/flutter/lib/shared/widgets/browser_context_menu_region.dart b/ui/flutter/lib/shared/widgets/browser_context_menu_region.dart new file mode 100644 index 00000000..3278ad41 --- /dev/null +++ b/ui/flutter/lib/shared/widgets/browser_context_menu_region.dart @@ -0,0 +1,81 @@ +import 'package:flutter/foundation.dart'; +import 'package:flutter/rendering.dart'; +import 'package:flutter/widgets.dart'; + +import 'browser_context_menu_region_stub.dart' + if (dart.library.js_interop) 'browser_context_menu_region_web.dart' + as platform; + +/// Keeps the browser menu out of an application's context-menu region on Web. +/// +/// Captured themes carry the policy into shadcn's menu and submenu overlays. +/// Overlay regions defer to their children so a full-screen positioning layer +/// does not suppress the browser menu over the rest of the page. +class BrowserContextMenuRegion extends StatelessWidget { + const BrowserContextMenuRegion({super.key, required this.child}); + + final Widget child; + + @override + Widget build(BuildContext context) { + if (!kIsWeb) return child; + return _ContextMenuTheme( + child: _ContextMenuRegion(behavior: HitTestBehavior.translucent, child: child), + ); + } +} + +class _ContextMenuTheme extends InheritedTheme { + const _ContextMenuTheme({required super.child}); + + @override + Widget wrap(BuildContext context, Widget child) { + return _ContextMenuTheme(child: _ContextMenuRegion(child: child)); + } + + @override + bool updateShouldNotify(_ContextMenuTheme oldWidget) => false; +} + +class _ContextMenuRegion extends SingleChildRenderObjectWidget { + const _ContextMenuRegion({this.behavior = HitTestBehavior.deferToChild, required super.child}); + + final HitTestBehavior behavior; + + @override + RenderObject createRenderObject(BuildContext context) { + return _RenderContextMenuRegion(View.of(context).viewId, behavior); + } + + @override + void updateRenderObject(BuildContext context, _RenderContextMenuRegion renderObject) { + renderObject + ..viewId = View.of(context).viewId + ..behavior = behavior; + } +} + +class _RenderContextMenuRegion extends RenderProxyBoxWithHitTestBehavior { + _RenderContextMenuRegion(this._viewId, HitTestBehavior behavior) : super(behavior: behavior); + + int _viewId; + + set viewId(int value) { + if (value == _viewId) return; + if (attached) platform.unregisterRegion(this, _viewId); + _viewId = value; + if (attached) platform.registerRegion(this, _viewId); + } + + @override + void attach(PipelineOwner owner) { + super.attach(owner); + platform.registerRegion(this, _viewId); + } + + @override + void detach() { + platform.unregisterRegion(this, _viewId); + super.detach(); + } +} diff --git a/ui/flutter/lib/shared/widgets/browser_context_menu_region_stub.dart b/ui/flutter/lib/shared/widgets/browser_context_menu_region_stub.dart new file mode 100644 index 00000000..ab9352f3 --- /dev/null +++ b/ui/flutter/lib/shared/widgets/browser_context_menu_region_stub.dart @@ -0,0 +1,5 @@ +import 'package:flutter/rendering.dart'; + +void registerRegion(RenderBox region, int viewId) {} + +void unregisterRegion(RenderBox region, int viewId) {} diff --git a/ui/flutter/lib/shared/widgets/browser_context_menu_region_web.dart b/ui/flutter/lib/shared/widgets/browser_context_menu_region_web.dart new file mode 100644 index 00000000..ba71a568 --- /dev/null +++ b/ui/flutter/lib/shared/widgets/browser_context_menu_region_web.dart @@ -0,0 +1,51 @@ +import 'dart:js_interop'; +import 'dart:ui' as ui; +import 'dart:ui_web' as ui_web; + +import 'package:flutter/rendering.dart'; +import 'package:web/web.dart' as web; + +final _regions = >{}; +JSFunction? _listener; + +void registerRegion(RenderBox region, int viewId) { + _regions.putIfAbsent(viewId, () => {}).add(region); + if (_listener != null) return; + _listener = _onContextMenu.toJS; + web.document.addEventListener('contextmenu', _listener, true.toJS); +} + +void unregisterRegion(RenderBox region, int viewId) { + final regions = _regions[viewId]; + regions?.remove(region); + if (regions?.isEmpty ?? false) _regions.remove(viewId); + if (_regions.isNotEmpty) return; + web.document.removeEventListener('contextmenu', _listener, true.toJS); + _listener = null; +} + +void _onContextMenu(web.MouseEvent event) { + if (!event.cancelable || event.defaultPrevented) return; + for (final view in ui.PlatformDispatcher.instance.views) { + final regions = _regions[view.viewId]; + if (regions == null) continue; + final host = ui_web.views.getHostElement(view.viewId) as web.HTMLElement?; + if (host == null || !event.composedPath().toDart.contains(host)) continue; + final bounds = host.getBoundingClientRect(); + if (bounds.width <= 0 || bounds.height <= 0) continue; + final x = event.clientX - bounds.left; + final y = event.clientY - bounds.top; + if (x < 0 || y < 0 || x >= bounds.width || y >= bounds.height) continue; + + // DOM client coordinates are CSS pixels. Map them through the public view + // host bounds rather than assuming a DPR, page origin or browser zoom. + final logicalSize = view.physicalSize / view.devicePixelRatio; + final position = Offset(x * logicalSize.width / bounds.width, y * logicalSize.height / bounds.height); + final result = HitTestResult(); + RendererBinding.instance.hitTestInView(result, position, view.viewId); + if (result.path.any((entry) => regions.contains(entry.target))) { + event.preventDefault(); + return; + } + } +} diff --git a/ui/flutter/test/features/tasks/presentation/widgets/task_context_menu_test.dart b/ui/flutter/test/features/tasks/presentation/widgets/task_context_menu_test.dart new file mode 100644 index 00000000..2f1bd555 --- /dev/null +++ b/ui/flutter/test/features/tasks/presentation/widgets/task_context_menu_test.dart @@ -0,0 +1,223 @@ +import 'package:flutter/foundation.dart'; +import 'package:flutter/gestures.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:gopeed/api/model/task.dart' as api_task; +import 'package:gopeed/features/tasks/domain/task_record.dart'; +import 'package:gopeed/features/tasks/presentation/widgets/task_context_menu.dart'; +import 'package:gopeed/shared/theme/app_component_themes.dart'; +import 'package:gopeed/shared/theme/app_theme.dart'; +import 'package:gopeed/shared/widgets/app_text_field.dart'; +import 'package:shadcn_flutter/shadcn_flutter.dart' as shad; + +import '../../../../support/context_menu_event.dart'; + +void main() { + testWidgets('task actions remain enabled by status and run once after reopening', (tester) async { + final calls = {}; + await tester.pumpWidget(_host(calls)); + await tester.pumpAndSettle(); + + await _open(tester, 'paused'); + expect(_button(tester, 'resume').enabled, isTrue); + expect(_button(tester, 'pause').enabled, isFalse); + await tester.tap(find.byKey(const ValueKey('task-context-resume'))); + await tester.pumpAndSettle(); + expect(calls, {'paused.resume': 1}); + + await _open(tester, 'paused'); + await tester.tap(find.byKey(const ValueKey('task-context-details'))); + await tester.pumpAndSettle(); + expect(calls['paused.details'], 1); + + await _open(tester, 'downloading'); + expect(_button(tester, 'resume').enabled, isFalse); + expect(_button(tester, 'pause').enabled, isTrue); + await tester.tap(find.byKey(const ValueKey('task-context-pause'))); + await tester.pumpAndSettle(); + expect(calls['downloading.pause'], 1); + + await _open(tester, 'paused'); + await tester.tap(find.byKey(const ValueKey('task-context-select'))); + await tester.pumpAndSettle(); + expect(calls['paused.select'], 1); + expect(calls.values, everyElement(1)); + }); + + testWidgets('task menus close on outside click and Escape and reopen after removal', (tester) async { + final calls = {}; + await tester.pumpWidget(_host(calls)); + await tester.pumpAndSettle(); + await _open(tester, 'paused'); + await tester.tapAt(_blank); + await tester.pumpAndSettle(); + expect(find.byKey(const ValueKey('task-context-details')), findsNothing); + expect(calls, isEmpty, reason: 'outside click only closes the menu'); + + await _open(tester, 'downloading'); + await tester.sendKeyEvent(LogicalKeyboardKey.escape); + await tester.pumpAndSettle(); + expect(find.byKey(const ValueKey('task-context-details')), findsNothing); + expect(calls, isEmpty, reason: 'Escape only closes the menu'); + + await tester.pumpWidget(_host(calls, showTasks: false)); + await tester.pumpAndSettle(); + await tester.pumpWidget(_host(calls)); + await tester.pumpAndSettle(); + await _open(tester, 'paused'); + await tester.tap(find.byKey(const ValueKey('task-context-details'))); + await tester.pumpAndSettle(); + expect(calls, {'paused.details': 1}); + }); + + testWidgets('Web cancels only task and popup contextmenu events and cleans up after removal', (tester) async { + final calls = {}; + await tester.pumpWidget(_host(calls)); + await tester.pumpAndSettle(); + expect(BrowserContextMenu.enabled, isTrue); + final firstPosition = tester.getCenter(find.byKey(const ValueKey('paused'))); + final secondPosition = tester.getCenter(find.byKey(const ValueKey('downloading'))); + final inputPosition = tester.getCenter(find.byType(AppTextField)); + + // Unlike a Flutter gesture, this observes cancellation of a DOM event. + expect(dispatchContextMenu(firstPosition, tester.view.viewId), isTrue); + expect(dispatchContextMenu(secondPosition, tester.view.viewId), isTrue); + expect(dispatchContextMenu(inputPosition, tester.view.viewId), isFalse); + expect(dispatchContextMenu(_blank, tester.view.viewId), isFalse); + + await _open(tester, 'paused'); + final popupPosition = tester.getCenter(find.byKey(const ValueKey('task-context-details'))); + expect(dispatchContextMenu(popupPosition, tester.view.viewId), isTrue); + expect(dispatchContextMenu(_blank, tester.view.viewId), isFalse); + + await tester.tap(find.byKey(const ValueKey('task-context-update-url'))); + // Submenus follow their anchor with a persistent ticker while open. + await tester.pump(); + await tester.pump(const Duration(milliseconds: 200)); + final manualUpdate = find.text('Update manually'); + expect(manualUpdate, findsOneWidget); + expect(dispatchContextMenu(tester.getCenter(manualUpdate), tester.view.viewId), isTrue); + + // shadcn keeps an already-open overlay alive when its anchor is removed. + // Its own captured region must survive independently of the task cards. + await tester.pumpWidget(_host(calls, showTasks: false)); + await tester.pump(const Duration(milliseconds: 200)); + expect(find.byType(TaskContextMenu), findsNothing); + expect(find.byKey(const ValueKey('task-context-details')), findsOneWidget); + expect(dispatchContextMenu(popupPosition, tester.view.viewId), isTrue); + expect(dispatchContextMenu(firstPosition, tester.view.viewId), isFalse); + + await tester.tapAt(_blank); + await tester.pumpAndSettle(); + expect(dispatchContextMenu(popupPosition, tester.view.viewId), isFalse); + expect(dispatchContextMenu(inputPosition, tester.view.viewId), isFalse); + expect(BrowserContextMenu.enabled, isTrue); + + await tester.pumpWidget(_host(calls, showTasks: false)); + await tester.pumpAndSettle(); + expect(dispatchContextMenu(firstPosition, tester.view.viewId), isFalse); + expect(dispatchContextMenu(secondPosition, tester.view.viewId), isFalse); + expect(dispatchContextMenu(inputPosition, tester.view.viewId), isFalse); + + await tester.pumpWidget(_host(calls)); + await tester.pumpAndSettle(); + expect(dispatchContextMenu(firstPosition, tester.view.viewId), isTrue); + await _open(tester, 'downloading'); + await tester.tap(find.byKey(const ValueKey('task-context-select'))); + await tester.pumpAndSettle(); + expect(calls, {'downloading.select': 1}); + expect(BrowserContextMenu.enabled, isTrue); + + await _open(tester, 'paused'); + await tester.pumpWidget(const SizedBox.expand()); + await tester.pumpAndSettle(); + expect(dispatchContextMenu(firstPosition, tester.view.viewId), isFalse); + expect(dispatchContextMenu(popupPosition, tester.view.viewId), isFalse); + expect(dispatchContextMenu(inputPosition, tester.view.viewId), isFalse); + }, skip: !kIsWeb); +} + +const _blank = Offset(700, 500); + +Future _open(WidgetTester tester, String id) async { + await tester.tapAt(tester.getCenter(find.byKey(ValueKey(id))), buttons: kSecondaryMouseButton); + await tester.pumpAndSettle(); + expect(find.byKey(const ValueKey('task-context-details')), findsOneWidget); +} + +shad.MenuButton _button(WidgetTester tester, String name) { + return tester.widget(find.byKey(ValueKey('task-context-$name'))); +} + +Widget _host(Map calls, {bool showTasks = true}) { + return shad.ShadcnApp( + disableBrowserContextMenu: false, + theme: AppTheme.light().copyWith(platform: () => TargetPlatform.windows), + materialTheme: AppTheme.materialLight(), + home: AppComponentThemes( + child: shad.Scaffold( + child: Stack( + children: [ + if (showTasks) + for (final (id, status, top) in [ + ('paused', TaskStatus.paused, 20.0), + ('downloading', TaskStatus.downloading, 100.0), + ]) + Positioned( + left: 20, + top: top, + child: TaskContextMenu( + key: ValueKey('menu-$id'), + task: TaskRecord( + id: id, + name: '$id.zip', + status: status, + downloaded: '0 B', + url: 'https://example.com/$id.zip', + storagePath: '/downloads/$id.zip', + files: const [], + uploading: false, + protocol: api_task.Protocol.http, + ), + actions: _actions(id, calls), + child: SizedBox( + key: ValueKey(id), + width: 240, + height: 64, + child: const ColoredBox(color: Color(0xFFFFFFFF)), + ), + ), + ), + const Positioned( + left: 20, + top: 220, + width: 240, + child: AppTextField(initialValue: 'https://example.com/download'), + ), + ], + ), + ), + ), + ); +} + +TaskContextActions _actions(String id, Map calls) { + void record(String name) => calls.update('$id.$name', (count) => count + 1, ifAbsent: () => 1); + return TaskContextActions( + selected: false, + allSelected: false, + listeningForUpdate: false, + onShowDetails: () => record('details'), + onOpenFile: () => record('openFile'), + onOpenDirectory: () => record('openDirectory'), + onBrowseFiles: () => record('browseFiles'), + onToggleSelectAll: () => record('selectAll'), + onToggleSelected: () => record('select'), + onPause: () => record('pause'), + onResume: () => record('resume'), + onDelete: () => record('delete'), + onUpdateUrl: () => record('updateUrl'), + onToggleUpdateListener: () => record('listen'), + ); +} diff --git a/ui/flutter/test/support/context_menu_event.dart b/ui/flutter/test/support/context_menu_event.dart new file mode 100644 index 00000000..090da831 --- /dev/null +++ b/ui/flutter/test/support/context_menu_event.dart @@ -0,0 +1 @@ +export 'context_menu_event_stub.dart' if (dart.library.js_interop) 'context_menu_event_web.dart'; diff --git a/ui/flutter/test/support/context_menu_event_stub.dart b/ui/flutter/test/support/context_menu_event_stub.dart new file mode 100644 index 00000000..b8969433 --- /dev/null +++ b/ui/flutter/test/support/context_menu_event_stub.dart @@ -0,0 +1,5 @@ +import 'dart:ui'; + +bool dispatchContextMenu(Offset position, int viewId) { + throw UnsupportedError('DOM contextmenu events require a browser'); +} diff --git a/ui/flutter/test/support/context_menu_event_web.dart b/ui/flutter/test/support/context_menu_event_web.dart new file mode 100644 index 00000000..9fa159f8 --- /dev/null +++ b/ui/flutter/test/support/context_menu_event_web.dart @@ -0,0 +1,25 @@ +import 'dart:ui'; +import 'dart:ui_web' as ui_web; + +import 'package:web/web.dart' as web; + +/// Exercises DOM cancellation, independently of Flutter's global menu flag. +/// Synthetic events cannot prove whether a browser's native menu is visible. +bool dispatchContextMenu(Offset position, int viewId) { + final view = PlatformDispatcher.instance.views.singleWhere((view) => view.viewId == viewId); + final host = ui_web.views.getHostElement(viewId) as web.HTMLElement; + final bounds = host.getBoundingClientRect(); + final logicalSize = view.physicalSize / view.devicePixelRatio; + final event = web.MouseEvent( + 'contextmenu', + web.MouseEventInit( + bubbles: true, + cancelable: true, + button: 2, + clientX: (bounds.left + position.dx * bounds.width / logicalSize.width).round(), + clientY: (bounds.top + position.dy * bounds.height / logicalSize.height).round(), + ), + ); + host.dispatchEvent(event); + return event.defaultPrevented; +}