Skip to content

Commit 8040ebc

Browse files
authored
Correctly implement PlatformViews' cursors on Web (flutter#174300)
Fix flutter#174246 Before: https://github.com/user-attachments/assets/33d2a476-d9c4-46e1-a5c5-ea8e602060d6 After: https://github.com/user-attachments/assets/383e4a01-4b8e-4c0e-a5a4-fa0d1fc28b67 This PR is not a result of a thorough fix for mouse cursors on all platforms, but simply matching the behavior of `SelectionArea` on Web to that on non-Web. A central question lies how a platform view widget should control the mouse cursor. On the surface, the answer is simple: it doesn't, and let the view content to control (hence the `MouseCursor.uncontrolled` constant). But what if the empty region of the view content doesn't control the cursor at all? This would be a problem even on non-Web, because if neither the view content nor the view controls the cursor on non-Web, then the user will find the cursor remaining the one used by the last region, which would be different when entering and when leaving, the same bug as flutter#174246 . On the other hand, what is it supposed to fall back to if the view content doesn't define a cursor? Is the default cursor the logically correct option? ... I decided that I did not want to dig too much into the details, and instead focus on this specific issue. I tried the repro app on macOS and found that the empty region of `SelectionArea` falls back to the widget behind it, the same behavior as `MouseCursor.defer`. Therefore in this PR I made `MouseCursor.uncontrolled` essentially the same as `.defer` on Web, matching the behavior across platforms. ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 9ff2767 commit 8040ebc

File tree

3 files changed

+160
-2
lines changed

3 files changed

+160
-2
lines changed

packages/flutter/lib/src/rendering/platform_view.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,7 @@ mixin _PlatformViewGestureMixin on RenderBox implements MouseTrackerAnnotation {
802802
PointerExitEventListener? get onExit => null;
803803

804804
@override
805-
MouseCursor get cursor => MouseCursor.uncontrolled;
805+
MouseCursor get cursor => kIsWeb ? MouseCursor.defer : MouseCursor.uncontrolled;
806806

807807
@override
808808
bool get validForMouseTracker => true;

packages/flutter/test/material/selection_area_test.dart

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,4 +582,73 @@ void main() {
582582
variant: TargetPlatformVariant.only(TargetPlatform.android),
583583
skip: kIsWeb, // [intended] on web only one selection handle can be dragged at a time.
584584
);
585+
586+
// Regression test for https://github.com/flutter/flutter/issues/174246 .
587+
// This is a control case against its Web counterpart in
588+
// html_element_view_test.dart.
589+
testWidgets('SelectionArea applies correct mouse cursors in its empty region', (
590+
WidgetTester tester,
591+
) async {
592+
final GlobalKey innerRegion = GlobalKey();
593+
await tester.pumpWidget(
594+
MaterialApp(
595+
debugShowCheckedModeBanner: false,
596+
home: Scaffold(
597+
// Region 1 (fullscreen)
598+
body: MouseRegion(
599+
cursor: SystemMouseCursors.grab,
600+
child: Center(
601+
child: Container(
602+
decoration: BoxDecoration(border: Border.all()),
603+
// Region 2 (SelectionArea)
604+
child: SelectionArea(
605+
child: Padding(
606+
padding: const EdgeInsetsGeometry.all(40),
607+
// Region 3 (inner MouseRegion)
608+
child: MouseRegion(
609+
key: innerRegion,
610+
cursor: SystemMouseCursors.forbidden,
611+
onHover: (_) {},
612+
child: Container(color: const Color(0xFFAA9933), width: 200, height: 50),
613+
),
614+
),
615+
),
616+
),
617+
),
618+
),
619+
),
620+
),
621+
);
622+
623+
await tester.pump();
624+
625+
const Offset region1 = Offset(10, 10);
626+
final Offset region2 = tester.getTopLeft(find.byKey(innerRegion)) - const Offset(3, 3);
627+
final Offset region3 = tester.getCenter(find.byKey(innerRegion));
628+
629+
final TestGesture gesture = await tester.startGesture(region1, kind: PointerDeviceKind.mouse);
630+
addTearDown(gesture.removePointer);
631+
expect(
632+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
633+
SystemMouseCursors.grab,
634+
);
635+
636+
await gesture.moveTo(region2);
637+
expect(
638+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
639+
SystemMouseCursors.grab,
640+
);
641+
642+
await gesture.moveTo(region3);
643+
expect(
644+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
645+
SystemMouseCursors.forbidden,
646+
);
647+
648+
await gesture.moveTo(region2);
649+
expect(
650+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
651+
SystemMouseCursors.grab,
652+
);
653+
}, skip: kIsWeb); // There's a Web version in html_element_view_test.dart
585654
}

packages/flutter/test/widgets/html_element_view_test.dart

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@
66
library;
77

88
import 'dart:async';
9+
import 'dart:ui' show PointerDeviceKind;
910
import 'dart:ui_web' as ui_web;
1011

12+
import 'package:flutter/material.dart';
1113
import 'package:flutter/rendering.dart';
1214
import 'package:flutter/services.dart';
13-
import 'package:flutter/widgets.dart';
1415
import 'package:flutter_test/flutter_test.dart';
1516
import 'package:web/web.dart' as web;
1617

@@ -44,6 +45,17 @@ void main() {
4445
return web.document.createElement(params['tagName']! as String);
4546
},
4647
);
48+
fakePlatformViewRegistry.registerViewFactory('Browser__WebContextMenuViewType__', (
49+
int viewId, {
50+
Object? params,
51+
}) {
52+
final web.HTMLElement htmlElement = web.document.createElement('div') as web.HTMLElement;
53+
htmlElement
54+
..style.width = '100%'
55+
..style.height = '100%'
56+
..classList.add('web-selectable-region-context-menu');
57+
return htmlElement;
58+
});
4759
});
4860

4961
group('HtmlElementView', () {
@@ -417,4 +429,81 @@ void main() {
417429
});
418430
});
419431
});
432+
433+
// Regression test for https://github.com/flutter/flutter/issues/174246
434+
// There is a control case for non-Web in selection_area_test.dart.
435+
testWidgets('SelectionArea applies correct mouse cursors in its empty region on Web', (
436+
WidgetTester tester,
437+
) async {
438+
final GlobalKey innerRegion = GlobalKey();
439+
await tester.pumpWidget(
440+
MaterialApp(
441+
debugShowCheckedModeBanner: false,
442+
home: Scaffold(
443+
// Region 1 (fullscreen)
444+
body: MouseRegion(
445+
cursor: SystemMouseCursors.grab,
446+
child: Center(
447+
child: Container(
448+
decoration: BoxDecoration(border: Border.all()),
449+
// Region 2 (SelectionArea)
450+
child: SelectionArea(
451+
child: Padding(
452+
padding: const EdgeInsetsGeometry.all(40),
453+
// Region 3 (inner MouseRegion)
454+
child: MouseRegion(
455+
key: innerRegion,
456+
cursor: SystemMouseCursors.forbidden,
457+
onHover: (_) {},
458+
child: Container(color: const Color(0xFFAA9933), width: 200, height: 50),
459+
),
460+
),
461+
),
462+
),
463+
),
464+
),
465+
),
466+
),
467+
);
468+
469+
// Initialize the HtmlElementView inside SelectionArea.
470+
await tester.pump();
471+
472+
// Ensure that the HtmlElementView is initialized.
473+
expect(
474+
find.byWidgetPredicate(
475+
(Widget widget) => widget.toString().contains('_PlatformViewPlaceHolder'),
476+
),
477+
findsNothing,
478+
);
479+
480+
const Offset region1 = Offset(10, 10);
481+
final Offset region2 = tester.getTopLeft(find.byKey(innerRegion)) - const Offset(3, 3);
482+
final Offset region3 = tester.getCenter(find.byKey(innerRegion));
483+
484+
final TestGesture gesture = await tester.startGesture(region1, kind: PointerDeviceKind.mouse);
485+
addTearDown(gesture.removePointer);
486+
expect(
487+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
488+
SystemMouseCursors.grab,
489+
);
490+
491+
await gesture.moveTo(region2);
492+
expect(
493+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
494+
SystemMouseCursors.grab,
495+
);
496+
497+
await gesture.moveTo(region3);
498+
expect(
499+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
500+
SystemMouseCursors.forbidden,
501+
);
502+
503+
await gesture.moveTo(region2);
504+
expect(
505+
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
506+
SystemMouseCursors.grab,
507+
);
508+
});
420509
}

0 commit comments

Comments
 (0)