[webview] Add macOS support in implementation package #6221

stuartmorgan · 2024-02-28T21:06:46Z

Adds initial support for macOS.

Known limitations:

Scroll APIs are not supported, and currently throw unimplemented. Longer term we could consider JS polyfills, but we'd probably want to make them opt-in since injecting JS into every page has some risks.
Setting the background color doesn't work since the view structure is different on macOS. We may be able to use a layer-backed view to support this in the future.

Since many use cases don't require these features, my preference is to land this without that support, rather than blocking on having full parity, and then the specific features can be added later based on community interest (demand and/or contributions).

This will not be landed until the next full stable release after 3.22, as 3.22 does not yet have gesture support for macOS platform views, and pushing this to users without gesture support would be very confusing.

Part of flutter/flutter#41725

stuartmorgan · 2024-02-28T21:08:53Z

packages/webview_flutter/webview_flutter_wkwebview/lib/src/ui_kit/ui_kit.dart

@bparrishMines I'm curious what you think of this approach to handle the fact that WKWebView has a different base class on iOS vs macOS.

I like this approach for sharing code between the platforms. Especially the part of creating a platform agnostic WKWebView class. I can also see this working if we need to add the NSView class for macOS. Although I'm not sure how this translates for the Pigeon wrapper generator since the Dart classes would become completely generated.

stuartmorgan · 2024-02-28T21:10:11Z

packages/webview_flutter/webview_flutter_wkwebview/lib/src/webkit_proxy.dart

- const WebKitProxy({
- this.createWebView = WKWebView.new,
+ WebKitProxy({
+ WebViewConstructor? createWebView,


@bparrishMines Unfortunately I had to make this non-const because of the Platform switch. Better ideas welcome!

You should be able to set this with a constant Function. Either by putting (Platform.isIOS ? WKWebViewIOS.new : WKWebViewMacOS.new); in a static method or a top-level function.

I've also have been waiting for this feature! : dart-lang/language#1048

Thanks for the tip! I've re-added the consts I removed.

Unfortunately I had to duplicate all the arguments, because I couldn't make a static function that returned Platform.isIOS ? WKWebViewIOS.new : WKWebViewMacOS.new and then call that because it still didn't want me calling the function in the const constructor, so I had to instead make it a wrapper that calls the right constructor each time, passing all the args through. (If I missed a way to make that work please let me know!)

stuartmorgan · 2024-02-28T21:11:02Z

packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart

- Future<String?> getCustomUserAgent() {
- return _webViewApi.getCustomUserAgentForInstances(this);
+/// The iOS version of a WKWebView.
+class WKWebViewIOS extends WKWebView implements UIView {


@bparrishMines This is the other half of how I'm handling the different parenting: the non-abstract classes are these subclasses which are different for the two platforms.

stuartmorgan · 2024-06-03T20:02:20Z

@bparrishMines @hellohuanlin This is ready for review. As noted in the PR description it won't land until the next stable, but since the PR is in a good state now we can work through reviews now so it's ready to go when the next stable arrives.

cbracken · 2024-06-04T17:57:02Z

..._flutter/webview_flutter_wkwebview/example/integration_test/legacy/webview_flutter_test.dart

+ },
+ // OGG playback is not supported on macOS, so the test data would need
+ // to be changed to support macOS.
+ skip: Platform.isMacOS);


nit: Is this indentation correct?

I don't know why the formatter does this; it's something to do with having a comment inline. We hit this with skip frequently, unfortunately.

The only alternative I can get the formatter to do is by adding a comma after isMacOS, but then it indents the entire test more, which is annoying for consistency with other tests.

hellohuanlin

Only reviewed the objc part and looks very clean

hellohuanlin · 2024-06-04T18:51:05Z

packages/webview_flutter/webview_flutter_wkwebview/darwin/Classes/FWFInstanceManager.m

 }

 - (BOOL)containsInstance:(nonnull NSObject *)instance {
 BOOL __block containsInstance;
 dispatch_sync(_lockQueue, ^{
- containsInstance = [self.identifiers objectForKey:instance];
+ containsInstance = [self.identifiers objectForKey:instance] != nil;


nit: (probably unrelated to this pr so no need to change): can just use pthread or NSLock if __block feels tedious to write.

Using queues as mutex locks feels a bit overkill although many people are doing it so 🤷. iirc i ran a benchmark super long time ago and locks (even the slow NSLock) are A LOT faster than queues.

cbenhagen · 2024-06-11T14:41:30Z

Can you add some information why setting a background is not supported on macOS? I guess most will want to make the background transparent. Is there a possibility to allow this special case?

stuartmorgan · 2024-06-11T15:27:57Z

Can you add some information why setting a background is not supported on macOS?

Sorry, I'm not sure what you are asking exactly. The PR description and my issue comment about the limitations of the PR discuss this; if you are looking for details that aren't there you'll need to be more specific.

Is there a possibility to allow this special case?

As always, anyone is welcome to submit PRs adding functionality they need.

bparrishMines

LGTM

This looks fine to land as is. I'll probably need to change the structure when the Swift ProxyApi generator is finished since I don't think it's compatible with this structure anymore. Mainly because the generated native file would reference the native class directly and the Dart class would be generated.

packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart

Co-authored-by: Maurice Parrish <[email protected]>

LouiseHsu · 2024-06-18T21:09:07Z

@stuartmorgan is this okay to merge or are there still outstanding issues?

stuartmorgan · 2024-06-19T00:08:14Z

This will not be landed until the next full stable release after 3.22, as 3.22 does not yet have gesture support for macOS platform views, and pushing this to users without gesture support would be very confusing.

Per the PR description:

This will not be landed until the next full stable release after 3.22, as 3.22 does not yet have gesture support for macOS platform views, and pushing this to users without gesture support would be very confusing.

stuartmorgan added 23 commits April 11, 2023 15:25

Add macOS support to example app

4903531

darwinify

2d412c0

Manually change includes; needs Pigeon fix

2f64c9d

Add missing entitlements

f96790c

Automatic changes from running

ce8c088

Get minimal PoC running

e3e5949

Add another TODO with issue reference

c5cd904

Merge branch 'main' into webview-macos

363a6d5

Remove the args hack

ee1f657

Update minimum version

18985c5

Update availability annotations

1912347

Merge branch 'main' into webview-macos

c70fb10

Remove the publish workaround

e99093d

Remove obsolete stub

3969dcc

Enable valuePublishedByPlugin codepath

6f703af

Comment some ifdefs

10df466

Change the way UIView removal works

29c5a6e

Use AppKitView

4c1ff63

Merge branch 'main' into webview-macos

56a8e5d

Merge branch 'main' into webview-macos

6f41e08

Resolve UIView issue

3c98078

Add back background color on iOS

aa15792

Merge branch 'main' into webview-macos

b530bf9

stuartmorgan requested a review from hellohuanlin as a code owner February 28, 2024 21:06

github-actions bot added platform-ios platform-macos p: webview_flutter labels Feb 28, 2024

stuartmorgan commented Feb 28, 2024

View reviewed changes

stuartmorgan added the waiting for stable update Can't be landed until functionality reaches the stable channel label May 21, 2024

stuartmorgan mentioned this pull request May 30, 2024

[webview_flutter] Add macOS support flutter/flutter#41725

Open

stuartmorgan added 6 commits May 30, 2024 11:47

Merge branch 'main' into webview-macos

88415cc

Skip unsupported integration tests

c5bca19

Missing licenses

8b75132

Move and add native unit tests

5b7bc0f

Version bump

3aaac0c

Fix unused function error

3b27394

This comment was marked as off-topic.

Sign in to view

Disable unit test parallelization

c14d5db

stuartmorgan marked this pull request as ready for review June 3, 2024 19:46

stuartmorgan requested a review from cbracken as a code owner June 3, 2024 19:46

stuartmorgan changed the title ~~[webview] Add macOS support [WIP]~~ [webview] Add macOS support Jun 3, 2024

stuartmorgan changed the title ~~[webview] Add macOS support~~ [webview] Add macOS support in implementation package Jun 3, 2024

Skip ogg tests

ec9a0ea

stuartmorgan added triage-macos Should be looked at in macOS triage triage-ios Should be looked at in iOS triage labels Jun 3, 2024

Skip legacy version of ogg tests as well

128a326

cbracken reviewed Jun 4, 2024

View reviewed changes

hellohuanlin approved these changes Jun 4, 2024

View reviewed changes

bparrishMines approved these changes Jun 13, 2024

View reviewed changes

packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart Outdated Show resolved Hide resolved

Fix doc comment

af5fd64

Co-authored-by: Maurice Parrish <[email protected]>

stuartmorgan removed triage-ios Should be looked at in iOS triage triage-macos Should be looked at in macOS triage labels Jun 20, 2024

hlwhl mentioned this pull request Jul 1, 2024

Keyboard does not work on open sites hlwhl/webview_cef#70

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[webview] Add macOS support in implementation package #6221

[webview] Add macOS support in implementation package #6221

stuartmorgan commented Feb 28, 2024 •

edited

Loading

stuartmorgan Feb 28, 2024

bparrishMines Mar 4, 2024

stuartmorgan Feb 28, 2024

bparrishMines Mar 4, 2024

stuartmorgan Apr 29, 2024

stuartmorgan Feb 28, 2024

This comment was marked as off-topic.

stuartmorgan commented Jun 3, 2024

cbracken Jun 4, 2024

stuartmorgan Jun 4, 2024

hellohuanlin left a comment

hellohuanlin Jun 4, 2024

cbenhagen commented Jun 11, 2024

stuartmorgan commented Jun 11, 2024

bparrishMines left a comment

LouiseHsu commented Jun 18, 2024

stuartmorgan commented Jun 19, 2024

[webview] Add macOS support in implementation package #6221

Are you sure you want to change the base?

[webview] Add macOS support in implementation package #6221

Conversation

stuartmorgan commented Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

stuartmorgan commented Jun 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellohuanlin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbenhagen commented Jun 11, 2024

stuartmorgan commented Jun 11, 2024

bparrishMines left a comment

Choose a reason for hiding this comment

LouiseHsu commented Jun 18, 2024

stuartmorgan commented Jun 19, 2024

stuartmorgan commented Feb 28, 2024 •

edited

Loading