Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: Support setting ICoreWebView2EnvironmentOptions8::ScrollBarStyle #1339

Closed
Themayu opened this issue Aug 13, 2024 · 15 comments · Fixed by #1344
Closed

Windows: Support setting ICoreWebView2EnvironmentOptions8::ScrollBarStyle #1339

Themayu opened this issue Aug 13, 2024 · 15 comments · Fixed by #1344
Labels
platform: Windows status: needs triage This issue or pull request needs to be investigated type: feature request

Comments

@Themayu
Copy link
Contributor

Themayu commented Aug 13, 2024

Is your feature request related to a problem? Please describe.

I need a way to set the WebView2 instance on Windows to use Fluent-style overlay scrollbars in Tauri. A member of the project has informed me on Discord that Wry exposes a way to set some platform-specific attributes by use of PlatformSpecificWebViewAttributes that are then used to configure settings in the WebView2 options and environment options.

Describe the solution you'd like

I propose adding a new field to PlatformSpecificWebViewAttributes on Windows, and a setter method to WebViewBuilderExtWindows to update this field. I'm unsure whether to use a boolean where true = fluent overlay scrollbars and false = default scrollbars, or an enum that exposes both Default and FluentOverlay. I feel the latter would be better for extensibility if Microsoft extend the list of scrollbar styles in the future.

I expect the API would look something like this (documentation omitted for brevity):

#[cfg(windows)]
#[derive(Clone, Copy, Default)]
pub enum ScrollbarStyle {
  #[default]
  Default,
  FluentOverlay,
}

...

#[cfg(windows)]
#[derive(Clone)]
pub(crate) struct PlatformSpecificWebViewAttributes {
  ...
  scrollbar_style: ScrollbarStyle,
}

#[cfg(windows)]
impl Default for PlatformSpecificWebViewAttributes {
  fn default() -> Self {
    Self {
      ...
      scrollbar_style: ScrollbarStyle::Default,
    }
  }
}

#[cfg(windows)]
pub trait WebViewBuilderExtWindows {
  ...
  fn with_scrollbar_style(self, style: ScrollbarStyle) -> Self;
}

#[cfg(windows)]
impl WebViewBuilderExtWindows for WebViewBuilder<'_> {
  ...
  fn with_scrollbar_style(self, style: ScrollbarStyle) -> Self {
    self.platform_specific.scrollbar_style = style;
    self
  }
}

Describe alternatives you've considered

I cannot use a browser flag for this, as per the Microsoft Documentation on WebView2 browser flags where it says that browser feature flags are subject to being removed or altered at any time.

Also, it was either this or adding and exposing a Windows-specific wrapper around all of ICoreWebView2EnvironmentOptions*. I feel that for now, this will do.

@tweidinger tweidinger added type: feature request platform: Windows status: needs triage This issue or pull request needs to be investigated labels Aug 13, 2024
@amrbashir
Copy link
Member

I'm unsure whether to use a boolean where true = fluent overlay scrollbars and false = default scrollbars, or an enum that exposes both Default and FluentOverlay. I feel the latter would be better for extensibility if Microsoft extend the list of scrollbar styles in the future.

An enum sounds fine

Do you plan to work on this in a PR? I can assign the issue to you in that case.

@Themayu
Copy link
Contributor Author

Themayu commented Aug 14, 2024

I'm fine with adding this myself.

@Themayu
Copy link
Contributor Author

Themayu commented Aug 14, 2024

Ah. I think I've encountered a bit of a problem: I can't figure out how I am going to expose this in Tauri, given that this is Windows-specific functionality with an enum that only exists in Wry when compiling for Windows.

@amrbashir
Copy link
Member

one step at a time, you only need to worry about exposing it as a method on WebviewBuilderExtWindows, then later we can open a PR in Tauri, that will call this method.

@Themayu
Copy link
Contributor Author

Themayu commented Aug 14, 2024

I have it exposed on WebviewBuilderExtWindows in my wry fork already, on the feat/windows-add-scrollbar-style-option branch. The reason I'm trying to figure out how to expose it in Tauri right now is so that I can test it, and make sure it's actually doing what I want it to.

@amrbashir
Copy link
Member

you can see this commit for inspiration tauri-apps/tauri@3dc38b1

@Themayu
Copy link
Contributor Author

Themayu commented Aug 14, 2024

...I just realised I don't need to expose it in Tauri to test it just yet. I can just do something dirty - force it inside a local patch of tauri_runtime_wry, then revert that when I'm done.

@Themayu
Copy link
Contributor Author

Themayu commented Aug 14, 2024

...

Well that's... very annoying.

Turns out that despite them being exposed, and despite the Microsoft documentation for ICoreWebView2EnvironmentOptions clearly showing the ability to cast CoreWebView2EnvironmentOptions to ICoreWebView2EnvironmentOptions4 and above, it turns out that webview2-com doesn't actually reflect the implementations of ICoreWebView2EnvironmentOptions4 or above on CoreWebView2EnvironmentOptions. So everything lines up according to the C++ documentation, but I'm completely unable to actually cast to ICoreWebView2EnvironmentOptions8 in order to set the scrollbar style.

I'm going to submit a request for the missing COM interfaces to be added.

@amrbashir
Copy link
Member

amrbashir commented Aug 14, 2024

webview2-com does have that https://docs.rs/webview2-com/latest/webview2_com/Microsoft/Web/WebView2/Win32/struct.ICoreWebView2EnvironmentOptions8.html , we just haven't updated the version in wry, probably because we need to update windows-sys and windows crate across our repos

gonna start working on that today

@Themayu
Copy link
Contributor Author

Themayu commented Aug 14, 2024

It has the interface defined. So does the version we are using here. It does not appear to have an implementation of that interface reflected on CoreWebView2EnvironmentOptions, which means we cannot cast to it.

@Themayu
Copy link
Contributor Author

Themayu commented Aug 14, 2024

I've opened a request in webview2-com: wravery/webview2-rs#30

@Themayu
Copy link
Contributor Author

Themayu commented Aug 15, 2024

@amrbashir Good news: The missing COM interface implementations were added to CoreWebView2EnvironmentOptions in [email protected], so this can go forward once Wry updates to the latest version.

Also, it turned out that the raw ICoreWebView2EnvironmentOptions4 had a problem, so it was replaced with a custom-made IFixedEnvironmentOptions4 instead.

@amrbashir
Copy link
Member

webview2-com is updated in #1341

@Themayu
Copy link
Contributor Author

Themayu commented Aug 16, 2024

Alright. I'll rebase onto that commit once it lands, then open a PR for both this and moving to using the CoreWebView2EnvironmentOptions API directly.

Themayu added a commit to Themayu/wry that referenced this issue Aug 18, 2024
Use the CoreWebView2EnvironmentOptions API wrapper when creating a
WebView2 environment on Windows, as opposed to using the
ICoreWebView2EnvironmentOptions* COM interfaces. The wrapper exposes
the interface methods with a more Rust-like API.

It may be possible to do this with ICoreWebView2Controller as well, but
I don't feel like experimenting with that right now. This is just an
improvement I realised I could make while implementing tauri-apps#1339.
Themayu added a commit to Themayu/wry that referenced this issue Aug 18, 2024
Use the CoreWebView2EnvironmentOptions API wrapper when creating a
WebView2 environment on Windows, as opposed to using the
ICoreWebView2EnvironmentOptions* COM interfaces. The wrapper exposes
the interface methods with a more Rust-like API.

It may be possible to do this with ICoreWebView2Controller as well, but
I don't feel like experimenting with that right now. This is just an
improvement I realised I could make while implementing tauri-apps#1339.

Signed-off-by: Jamie Ridding <[email protected]>
Themayu added a commit to Themayu/wry that referenced this issue Aug 19, 2024
Use the CoreWebView2EnvironmentOptions API wrapper when creating a
WebView2 environment on Windows, as opposed to using the
ICoreWebView2EnvironmentOptions* COM interfaces. The wrapper exposes
the interface methods with a more Rust-like API.

It may be possible to do this with ICoreWebView2Controller as well, but
I don't feel like experimenting with that right now. This is just an
improvement I realised I could make while implementing tauri-apps#1339.

Signed-off-by: Jamie Ridding <[email protected]>
pewsheen pushed a commit that referenced this issue Aug 21, 2024
…1344)

* Windows: Use `CoreWebView2EnvironmentOptions` API.

Use the CoreWebView2EnvironmentOptions API wrapper when creating a
WebView2 environment on Windows, as opposed to using the
ICoreWebView2EnvironmentOptions* COM interfaces. The wrapper exposes
the interface methods with a more Rust-like API.

It may be possible to do this with ICoreWebView2Controller as well, but
I don't feel like experimenting with that right now. This is just an
improvement I realised I could make while implementing #1339.

Signed-off-by: Jamie Ridding <[email protected]>

* Add with_scroll_bar_style to WebViewBuilderExtWindows

Signed-off-by: Jamie Ridding <[email protected]>

* Add change file.

Signed-off-by: Jamie Ridding <[email protected]>

* Adjust change type in change file.

I used the wrong change type for wry in 1344-with_scroll_bar_style.
This commit upgrades the change type from a patch to a minor change.

Signed-off-by: Jamie Ridding <[email protected]>

* I forgot to run cargo fmt -_-

Signed-off-by: Jamie Ridding <[email protected]>

---------

Signed-off-by: Jamie Ridding <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: Windows status: needs triage This issue or pull request needs to be investigated type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants