This repository was archived by the owner on Feb 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[webview_flutter] - Added WebViewSetting option for iOS #4339
Closed
Closed
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
f7e472f
Added dependency overrides
7a813c5
Added `limitsNavigationsToAppBoundDomains` to `CreationParams`
b40866d
Added logic to handle limitsNavigationsToAppBoundDomains when webview…
f1e4a2b
Added limitsNavigationsToAppBoundDomains to `WebSettings`
28b95b6
Moved code
f059537
Added limitsNavigationsToAppBoundDomains to example WebView
defc442
Added native unit test for limitsNavigationsToAppBoundDomains
66b7cbb
Added flutter unit test to make sure limitsNavigationsToAppBoundDomai…
dd012d3
Added documentation
ba48874
added dependency overrides
b27b572
Updated changelog and pubspec
dc456ef
Formatting
97f11f1
Merge branch 'master' of github.com:flutter/plugins
bb4b20f
Merge branch 'master' of github.com:flutter/plugins
b091857
Merge branch 'master' of github.com:flutter/plugins
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 4 additions & 0 deletions
4
packages/webview_flutter/webview_flutter_platform_interface/CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding platform-specific arguments to this list is not a direction I want to take if we can possibly avoid it; it could get out of hand very quickly.
@bparrishMines @mvanbeusekom You're both more familiar than I am with the overall API of this plugin; do either of you have thoughts on how to introduce platform-specific extensions here?
I'm wondering if it would make sense to add a creation parameters class that's created by a factory, implemented by the implementing packages, so that they could return subclasses of the settings object that have extensions. Then clients could do platform-specific casting to access settings like this one, similar to the IAP extensions.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the long term, I believe that yes we could use casting to add cleaner support for very specific platform features like this. For example, the API would look something like:
However, this can only be done after the iOS platform implementation is transitioned to pigeon and we update the platform interface. After #4503 lands, this should be possible for Android. So we may want to hold off on adding this feature until iOS is also transitioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I'm missing something with your example @bparrishMines, but
limitsNavigationsToAppBoundDomainsis not a setting that can be changed after theWebViewhas been initialized. It needs to be configured during the initialize process of theFLTWebViewControllerobject. And if I understand it correctlyonWebViewCreatedis called after both the WebView Configuration and the WebView is done initializingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I hadn't realized it was part of initialization. Then, I agree with @stuartmorgan that we could expose a
CreationParamsclass that each platform can add additional fields too. A solution that could look something like this in the platform interface:@stuartmorgan Is this what you were considering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks pretty much like I was thinking (except that we'd probably want the platform interface class to return the base class rather than throw unimplemented, so that only platforms that want extra parameters need to implement it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartmorgan is this something I should implement, or should I wait for it to be implemented before adding my changes? Also, how would this look in an app where I might target multiple platforms and want toggle options for multiple platforms?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not something there's an existing plan to implement, so you should go ahead and implement it. I would start with trying it as part of this PR; we could do it as a separate PR that lands first, but it's probably useful to build (and review) in conjunction with this use as a test case to make sure it's working as we want it to end to end. (If it turns out to be more complex than anticipated for some reason, it could be split out later.)
Basically like the example shown above, but with the settings object being cast, not the controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, but as @bparrishMines mentioned, it should wait a couple of weeks since the restructuring that's happening in the webview implementation will probably interact with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartmorgan Sounds good, is there an active issue or PR for the iOS transition to pigeon that I can follow to know when this change is landed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There wasn't, but really should be. I filed flutter/flutter#93732 (Android is in progress now and nearing completion, iOS will follow after).