[webview_flutter] Adds support to listen to url changes #3313

bparrishMines · 2023-02-27T17:15:51Z

Transfer of flutter/plugins#7113 (All the comments of that PR have been addressed in this one)

This PR follows the procedure for changing federated plugins: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins

~~This is the aggregate solution for flutter/flutter#27729 and is NOT intended to be submitted. This will be split into PRs for each package after approval of this PR.~~ All split PRs have been merged.

This adds the PlatformNavigationDelegate.setOnUrlChange(UrlChange) method to track url changes.

Android adds support for this with the WebViewClient.doUpdateVisitedHistory

iOS adds support by observing the value WKWebView.URL. Since the WKWebView.URL would pass back an NSURL to the observeValue callback, the observeValue callback would need to distinguish the difference when returning an int or an identifier. So the ObjectOrIdentifier data class was created to handle this. Alternatives considered:

Convert NSURL to an NSString. This would work for this situation, but wouldn't work for a class that can't be easily converted to a String.
Create a separate observeValue callback method for returning an identifier. Potentially named observeIdentifier. This doesn't work for returning a list of Objects.

Fixes flutter/flutter#27729

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

cyanglaz · 2023-02-27T21:49:35Z

packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFURLHostApi.m

+                                   error:(FlutterError *_Nullable __autoreleasing *_Nonnull)error {
+  NSURL *instance = [self urlForIdentifier:identifier error:error];
+  if (*error) {
+    return nil;


Should we handle the error?

The pigeon host API is calling this method, so the error should be passed back to Dart. The error in *error could be set by the method above.

bparrishMines · 2023-02-28T18:05:17Z

packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFGeneratedWebKitApis.m

@@ -1673,46 +1707,10 @@ @implementation FWFNSObjectFlutterApiCodecReader
 - (nullable id)readValueOfType:(UInt8)type {
  switch (type) {
    case 128:
-      return [FWFNSErrorData fromList:[self readValue]];


These are removed because the class method observeValue now returns an ObjectOrIdentifier. When it returned an Object, all of these were included as possible return values.

cyanglaz

iOS looks good to me

stuartmorgan

(Transferring my high-level LGTM from the other PR to keep GitHub status up to date with reality.)

bparrishMines · 2023-04-13T17:18:20Z

packages/webview_flutter/webview_flutter/lib/src/navigation_delegate.dart

  NavigationDelegate.fromPlatform(
    this.platform, {
    this.onNavigationRequest,
    this.onPageStarted,
    this.onPageFinished,
    this.onProgress,
    this.onWebResourceError,
+    void Function(UrlChange change)? onUrlChange,


This was not added as a field of the clas because there is a chance that the field can be different from the actual value. For example, a user may set this value and then call NavigationDelegate.platform.setOnUrlChange:

final NavigationDelegate delegate = NavigationDelegate(onUrlChange: (_) {}); delegate.platform.setUrlChange((_) {});

This is true for all the other callbacks, but changing them would be a breaking change.

This is true for all the other callbacks, but changing them would be a breaking change.

Couldn't we replace the final fields with getters, and have the getters return the platform version of the field, to preserve API compatibility while removing the potential for them to become out of sync?

(Doesn't need to be in this PR.)

@mvanbeusekom suggested something similar by making the callback method setters in PlatformNavigationDelegate fields instead of methods. I decided against it initially because using a method allowed us to throw an UnimplementedError for unsupported callback methods. But we could add getters to the interface as a mix of both solutions.

This wouldn't be possible in this PR since it is something we would have to add to the platform interface first anyways.

stuartmorgan

LGTM!

stuartmorgan · 2023-04-18T14:59:07Z

packages/webview_flutter/webview_flutter/lib/src/navigation_delegate.dart

  NavigationDelegate.fromPlatform(
    this.platform, {
    this.onNavigationRequest,
    this.onPageStarted,
    this.onPageFinished,
    this.onProgress,
    this.onWebResourceError,
+    void Function(UrlChange change)? onUrlChange,


This is true for all the other callbacks, but changing them would be a breaking change.

Couldn't we replace the final fields with getters, and have the getters return the platform version of the field, to preserve API compatibility while removing the potential for them to become out of sync?

(Doesn't need to be in this PR.)

…ter/packages#3313)

flutter/packages@faf53fb...88591be 2023-04-19 [email protected] Roll Flutter from 42fb0b2 to 3476b96 (20 revisions) (flutter/packages#3760) 2023-04-19 49699333+dependabot[bot]@users.noreply.github.com Bump cirrusci/flutter from `794fbbc` to `d99b1ba` in /.ci (flutter/packages#3724) 2023-04-18 [email protected] [webview_flutter] Adds support to listen to url changes (flutter/packages#3313) 2023-04-18 [email protected] Roll Flutter from 15cb1f8 to 42fb0b2 (19 revisions) (flutter/packages#3756) 2023-04-18 49699333+dependabot[bot]@users.noreply.github.com [webview]: Bump com.android.tools.build:gradle from 7.2.2 to 8.0.0 in /packages/webview_flutter/webview_flutter_android/android (flutter/packages#3739) 2023-04-18 [email protected] [local_auth] Convert Android to Pigeon (flutter/packages#3748) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Transfer of flutter/plugins#7113 (All the comments of that PR have been addressed in this one) This PR follows the procedure for changing federated plugins: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins ~~This is the aggregate solution for flutter/flutter#27729 and is NOT intended to be submitted. This will be split into PRs for each package after approval of this PR.~~ All split PRs have been merged. This adds the `PlatformNavigationDelegate.setOnUrlChange(UrlChange)` method to track url changes. Android adds support for this with the [WebViewClient.doUpdateVisitedHistory](https://developer.android.com/reference/android/webkit/WebViewClient#doUpdateVisitedHistory(android.webkit.WebView,%20java.lang.String,%20boolean)) iOS adds support by observing the value [WKWebView.URL](https://developer.apple.com/documentation/webkit/wkwebview/1415005-url?language=objc). Since the WKWebView.URL would pass back an [NSURL](https://developer.apple.com/documentation/foundation/nsurl?language=objc) to the [observeValue](https://developer.apple.com/documentation/objectivec/nsobject/1416553-observevalueforkeypath?language=objc) callback, the `observeValue` callback would need to distinguish the difference when returning an `int` or an identifier. So the `ObjectOrIdentifier` data class was created to handle this. Alternatives considered: * Convert `NSURL `to an `NSString`. This would work for this situation, but wouldn't work for a class that can't be easily converted to a `String`. * Create a separate `observeValue `callback method for returning an identifier. Potentially named `observeIdentifier`. This doesn't work for returning a list of Objects. Fixes flutter/flutter#27729

bparrishMines added 5 commits February 24, 2023 16:09

platform interface

dbc0e36

android

14860c3

ios

9bf1f7f

app facing

9579d25

Merge branch 'main' of github.com:flutter/packages into url_change_tres

88766ab

bparrishMines requested a review from cyanglaz as a code owner February 27, 2023 17:15

github-actions bot added p: webview_flutter platform-android platform-ios labels Feb 27, 2023

cyanglaz reviewed Feb 27, 2023

View reviewed changes

bparrishMines added 2 commits February 28, 2023 11:37

Merge branch 'main' of github.com:flutter/packages into url_change_tres

e0dbc46

fix unused parameter

be6f453

bparrishMines commented Feb 28, 2023

View reviewed changes

bparrishMines added 2 commits February 28, 2023 13:16

add history test and some fixes

9904b49

undo slight mock changes

d3ba003

bparrishMines changed the title ~~Url change tres~~ [webview_flutter] Adds support to listen to url changes Feb 28, 2023

cyanglaz approved these changes Feb 28, 2023

View reviewed changes

bparrishMines mentioned this pull request Feb 28, 2023

[webview_flutter_platform_interface] Adds support to track url changes in the platform interface #3323

Merged

11 tasks

bparrishMines requested a review from stuartmorgan February 28, 2023 19:27

stuartmorgan requested changes Mar 9, 2023

View reviewed changes

bparrishMines added 4 commits April 6, 2023 10:17

Merge branch 'main' of github.com:flutter/packages into url_change_tres

c4a9a73

remove dep overrides

b493548

create webview on android

08d1c2c

add android and wkwebview deps back

3946118

bparrishMines mentioned this pull request Apr 6, 2023

[webview_flutter_android] [webview_flutter_wkwebview] Adds support for PlatformNavigationDelegate.onUrlChange #3653

Merged

11 tasks

bparrishMines added 3 commits April 13, 2023 12:13

Merge branch 'main' of github.com:flutter/packages into url_change_tres

3723132

undo changes

b33679b

docs fix

571bce4

github-actions bot removed platform-ios platform-android labels Apr 13, 2023

bparrishMines added 3 commits April 13, 2023 13:01

version bumps

ca49681

remove overrides

4517615

change template name

516cec3

bparrishMines commented Apr 13, 2023

View reviewed changes

bparrishMines requested a review from stuartmorgan April 13, 2023 17:19

Merge branch 'main' of github.com:flutter/packages into url_change_tres

9079fa4

stuartmorgan approved these changes Apr 18, 2023

View reviewed changes

bparrishMines added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 18, 2023

auto-submit bot merged commit 9342f5e into flutter:main Apr 18, 2023

bparrishMines deleted the url_change_tres branch April 18, 2023 19:20

engine-flutter-autoroll mentioned this pull request Apr 19, 2023

Roll Packages from faf53fb3998c to c6f207ca73ce (5 revisions) flutter/flutter#125136

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 19, 2023

9342f5e [webview_flutter] Adds support to listen to url changes (flut…

d4b606a

…ter/packages#3313)

engine-flutter-autoroll mentioned this pull request Apr 19, 2023

Roll Packages from faf53fb3998c to c6f207ca73ce (5 revisions) flutter/flutter#125149

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 19, 2023

9342f5e [webview_flutter] Adds support to listen to url changes (flut…

ba5d9d8

…ter/packages#3313)

engine-flutter-autoroll mentioned this pull request Apr 19, 2023

Roll Packages from faf53fb3998c to 88591becfb3f (6 revisions) flutter/flutter#125153

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 19, 2023

9342f5e [webview_flutter] Adds support to listen to url changes (flut…

1ecf249

…ter/packages#3313)

reidbaker mentioned this pull request Apr 25, 2023

[various] update agp and gradle for all examples in packages #3822

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[webview_flutter] Adds support to listen to url changes #3313

[webview_flutter] Adds support to listen to url changes #3313

bparrishMines commented Feb 27, 2023 •

edited

Loading

cyanglaz Feb 27, 2023

bparrishMines Feb 28, 2023

bparrishMines Feb 28, 2023

cyanglaz left a comment

stuartmorgan left a comment

bparrishMines Apr 13, 2023

stuartmorgan Apr 18, 2023

bparrishMines Apr 18, 2023

stuartmorgan left a comment

stuartmorgan Apr 18, 2023

[webview_flutter] Adds support to listen to url changes #3313

[webview_flutter] Adds support to listen to url changes #3313

Conversation

bparrishMines commented Feb 27, 2023 • edited Loading

Pre-launch Checklist

cyanglaz Feb 27, 2023

Choose a reason for hiding this comment

bparrishMines Feb 28, 2023

Choose a reason for hiding this comment

bparrishMines Feb 28, 2023

Choose a reason for hiding this comment

cyanglaz left a comment

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

bparrishMines Apr 13, 2023

Choose a reason for hiding this comment

stuartmorgan Apr 18, 2023

Choose a reason for hiding this comment

bparrishMines Apr 18, 2023

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan Apr 18, 2023

Choose a reason for hiding this comment

bparrishMines commented Feb 27, 2023 •

edited

Loading