Skip to content
This repository was archived by the owner on Feb 12, 2025. It is now read-only.

Commit fd2211f

Browse files
committed
[web] Fix HashUrlStrategy.addPopStateListener (flutter#41384)
During a JS-interop refactor, we introduced a small bug in the `addPopStateListener` method of the `HashUrlStrategy` object (web). This wasn't caught before because the existing tests were mocking the refactored code. ## Issues Fixes: flutter/flutter#125228 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent a645b36 commit fd2211f

File tree

2 files changed

+52
-3
lines changed

2 files changed

+52
-3
lines changed

lib/web_ui/lib/src/engine/navigation/url_strategy.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ class HashUrlStrategy extends ui_web.UrlStrategy {
3535

3636
@override
3737
ui.VoidCallback addPopStateListener(ui_web.PopStateListener fn) {
38-
final DomEventListener wrappedFn = createDomEventListener(fn);
38+
final DomEventListener wrappedFn = createDomEventListener((DomEvent event) {
39+
// `fn` expects `event.state`, not a `DomEvent`.
40+
fn((event as DomPopStateEvent).state);
41+
});
3942
_platformLocation.addPopStateListener(wrappedFn);
4043
return () => _platformLocation.removePopStateListener(wrappedFn);
4144
}

lib/web_ui/test/engine/history_test.dart

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@
77
library;
88

99
import 'dart:async';
10+
import 'dart:js_interop'
11+
show JSExportedDartFunction, JSExportedDartFunctionToFunction;
1012

1113
import 'package:quiver/testing/async.dart';
1214
import 'package:test/bootstrap/browser.dart';
1315
import 'package:test/test.dart';
14-
import 'package:ui/src/engine.dart' show DomEventListener, window;
16+
import 'package:ui/src/engine.dart' show window;
1517
import 'package:ui/src/engine/browser_detection.dart';
18+
import 'package:ui/src/engine/dom.dart'
19+
show DomEvent, DomEventListener, createDomPopStateEvent;
1620
import 'package:ui/src/engine/navigation.dart';
1721
import 'package:ui/src/engine/services.dart';
1822
import 'package:ui/src/engine/test_embedding.dart';
@@ -647,6 +651,31 @@ void testMain() {
647651
location.hash = '#';
648652
expect(strategy.getPath(), '/');
649653
});
654+
655+
test('addPopStateListener fn unwraps DomPopStateEvent state', () {
656+
final HashUrlStrategy strategy = HashUrlStrategy(location);
657+
const String expected = 'expected value';
658+
final List<Object?> states = <Object?>[];
659+
660+
// Put the popStates received from the `location` in a list
661+
strategy.addPopStateListener(states.add);
662+
663+
// Simulate a popstate with a null state:
664+
location.debugTriggerPopState(null);
665+
666+
expect(states, hasLength(1));
667+
expect(states[0], isNull);
668+
669+
// Simulate a popstate event with `expected` as its 'state'.
670+
location.debugTriggerPopState(expected);
671+
672+
expect(states, hasLength(2));
673+
final Object? state = states[1];
674+
expect(state, isNotNull);
675+
// flutter/flutter#125228
676+
expect(state, isNot(isA<DomEvent>()));
677+
expect(state, expected);
678+
});
650679
});
651680
}
652681

@@ -694,15 +723,32 @@ class TestPlatformLocation extends PlatformLocation {
694723
@override
695724
dynamic state;
696725

726+
List<DomEventListener> popStateListeners = <DomEventListener>[];
727+
697728
@override
698729
String get pathname => throw UnimplementedError();
699730

700731
@override
701732
String get search => throw UnimplementedError();
702733

734+
/// Calls all the registered `popStateListeners` with a 'popstate'
735+
/// event with value `state`
736+
void debugTriggerPopState(Object? state) {
737+
final DomEvent event = createDomPopStateEvent(
738+
'popstate',
739+
<Object, Object>{
740+
if (state != null) 'state': state,
741+
},
742+
);
743+
for (final DomEventListener listener in popStateListeners) {
744+
final Function fn = (listener as JSExportedDartFunction).toDart;
745+
fn(event);
746+
}
747+
}
748+
703749
@override
704750
void addPopStateListener(DomEventListener fn) {
705-
throw UnimplementedError();
751+
popStateListeners.add(fn);
706752
}
707753

708754
@override

0 commit comments

Comments
 (0)