[web] Fix HashUrlStrategy.addPopStateListener (flutter/engine#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: https://github.com/flutter/flutter/issues/125228

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
David Iglesias
2023-04-21 10:26:31 -07:00
committed by GitHub
parent 549dd82f2f
commit dfd9ef5e60
2 changed files with 52 additions and 3 deletions

View File

@@ -35,7 +35,10 @@ class HashUrlStrategy extends ui_web.UrlStrategy {
@override
ui.VoidCallback addPopStateListener(ui_web.PopStateListener fn) {
final DomEventListener wrappedFn = createDomEventListener(fn);
final DomEventListener wrappedFn = createDomEventListener((DomEvent event) {
// `fn` expects `event.state`, not a `DomEvent`.
fn((event as DomPopStateEvent).state);
});
_platformLocation.addPopStateListener(wrappedFn);
return () => _platformLocation.removePopStateListener(wrappedFn);
}

View File

@@ -7,12 +7,16 @@
library;
import 'dart:async';
import 'dart:js_interop'
show JSExportedDartFunction, JSExportedDartFunctionToFunction;
import 'package:quiver/testing/async.dart';
import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';
import 'package:ui/src/engine.dart' show DomEventListener, window;
import 'package:ui/src/engine.dart' show window;
import 'package:ui/src/engine/browser_detection.dart';
import 'package:ui/src/engine/dom.dart'
show DomEvent, DomEventListener, createDomPopStateEvent;
import 'package:ui/src/engine/navigation.dart';
import 'package:ui/src/engine/services.dart';
import 'package:ui/src/engine/test_embedding.dart';
@@ -647,6 +651,31 @@ void testMain() {
location.hash = '#';
expect(strategy.getPath(), '/');
});
test('addPopStateListener fn unwraps DomPopStateEvent state', () {
final HashUrlStrategy strategy = HashUrlStrategy(location);
const String expected = 'expected value';
final List<Object?> states = <Object?>[];
// Put the popStates received from the `location` in a list
strategy.addPopStateListener(states.add);
// Simulate a popstate with a null state:
location.debugTriggerPopState(null);
expect(states, hasLength(1));
expect(states[0], isNull);
// Simulate a popstate event with `expected` as its 'state'.
location.debugTriggerPopState(expected);
expect(states, hasLength(2));
final Object? state = states[1];
expect(state, isNotNull);
// flutter/flutter#125228
expect(state, isNot(isA<DomEvent>()));
expect(state, expected);
});
});
}
@@ -694,15 +723,32 @@ class TestPlatformLocation extends PlatformLocation {
@override
dynamic state;
List<DomEventListener> popStateListeners = <DomEventListener>[];
@override
String get pathname => throw UnimplementedError();
@override
String get search => throw UnimplementedError();
/// Calls all the registered `popStateListeners` with a 'popstate'
/// event with value `state`
void debugTriggerPopState(Object? state) {
final DomEvent event = createDomPopStateEvent(
'popstate',
<Object, Object>{
if (state != null) 'state': state,
},
);
for (final DomEventListener listener in popStateListeners) {
final Function fn = (listener as JSExportedDartFunction).toDart;
fn(event);
}
}
@override
void addPopStateListener(DomEventListener fn) {
throw UnimplementedError();
popStateListeners.add(fn);
}
@override