Skip to content

[v2 mac] Allow to specify webview preferences #2937

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

Conversation

fkhadra
Copy link
Contributor

@fkhadra fkhadra commented Sep 23, 2023

Description

This PR allows to set some of the preferences for the Webview on osx, notably tabFocusesLinks and textInteractionEnabled. By default, tabFocusesLinks is disabled, preventing the user to select links with the tab key. One could use javascript to workaround this issue, but it's very brittle and error prone. Having a way to turn this setting on increase the accessibility of the app.

Fixes #2919

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

I've tested the changes locally with a project I'm currently working on by adding a replace statement in my go.mod to point to the version with my changes. I've added a recording:

wails-webkit-preference.mp4
  • during the first app launch, tabFocusesLinks is enabled, pressing tab moves the focus ring to overview, settings, etc... Also I'm not unable to select any text as textInteractionEnabled is disabled
  • after changing the settings, pressing tabs does not focus overview, settings, etc..., only button are focusable. And text is selectable again

In the recording, to set the preferences, I was using u.Bool directly, it requires end user to have the package in their dependencies.

require (
	github.com/leaanthony/u v1.0.1
	github.com/wailsapp/wails/v2 v2.6.0
	github.com/zalando/go-keyring v0.2.3
	go.uber.org/zap v1.26.0
	golang.org/x/net v0.15.0
)

It's not an issue per se but I've decided to use a more user friendly solution to avoid exposing implementation details to the end user. I've added 2 functions: mac.EnablePreference: u.Bool and mac.DisablePreference.

Preferences: &mac.Preferences{
		TabFocusesLinks:           mac.EnablePreference(),
		TextInteractionEnabled: mac.DisablePreference(),
}

Regarding the name of the preferences, I've used the one from apple documentation.
The repository I've used in the recording is private but I can setup a simple app if needed.

  • Windows
  • macOS
  • Linux

Test Configuration

# Wails
Version | v2.6.0

# System
┌─────────────────────────┐
| OS           | MacOS    |
| Version      | 13.5.2   |
| ID           | 22G91    |
| Go Version   | go1.21.0 |
| Platform     | darwin   |
| Architecture | arm64    |
└─────────────────────────┘

# Dependencies
┌──────────────────────────────────────────────────────────────────┐
| Dependency                | Package Name | Status    | Version   |
| Xcode command line tools  | N/A          | Installed | 2397      |
| Nodejs                    | N/A          | Installed | 18.17.0   |
| npm                       | N/A          | Installed | 9.6.7     |
| *Xcode                    | N/A          | Available |           |
| *upx                      | N/A          | Installed | upx 4.1.0 |
| *nsis                     | N/A          | Available |           |
└──────────────────── * - Optional Dependency ─────────────────────┘

# Diagnosis
Optional package(s) installation details:
  - Xcode: Available at https://apps.apple.com/us/app/xcode/id497799835
  - nsis : More info at https://wails.io/docs/guides/windows-installer/

 SUCCESS  Your system is ready for Wails development!

 ♥   If Wails is useful to you or your company, please consider sponsoring the project:
https://github.com/sponsors/leaanthony

Checklist:

  • I have updated website/src/pages/changelog.mdx with details of this PR
  • My code follows the general coding style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Member

@leaanthony leaanthony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR! 🙏
There's 1 thing we need to change and a couple of things to consider.

@leaanthony
Copy link
Member

There's an interesting error here @fkhadra - any ideas? It does appear to work when i use it in an app. Just wondering what is affecting this test.

@fkhadra
Copy link
Contributor Author

fkhadra commented Sep 24, 2023

Looking into it.

@fkhadra
Copy link
Contributor Author

fkhadra commented Sep 24, 2023

Running the test suites locally yielded no errors. Still trying to understand what's going on 🤔
Screenshot 2023-09-24 at 13 06 01

@fkhadra
Copy link
Contributor Author

fkhadra commented Sep 24, 2023

I'm using an M1 mac btw, I'm wondering is the issue can be caused by the CPU arch.

Edit: I think I have a lead!

@fkhadra
Copy link
Contributor Author

fkhadra commented Sep 24, 2023

Regarding my latest commit, why I've changed BOOL to bool.
https://opensource.apple.com/source/objc4/objc4-706/runtime/objc.h.auto.html

/// Type to represent a boolean value.
#if (TARGET_OS_IPHONE && __LP64__)  ||  TARGET_OS_WATCH
#define OBJC_BOOL_IS_BOOL 1
typedef bool BOOL;
#else
#define OBJC_BOOL_IS_CHAR 1
typedef signed char BOOL;  // ---> the guilty one
// BOOL is explicitly signed so @encode(BOOL) == "c" rather than "C" 
// even if -funsigned-char is used.
#endif

@leaanthony
Copy link
Member

I tested on M2 which I guess is why it worked. Nice find.

@fkhadra
Copy link
Contributor Author

fkhadra commented Sep 24, 2023

Yes I'll sleep a bit smarter than I woke up 😄. I was wondering if you'd like me to address video tag can not be full screen as well in this PR or I can open another one when this one is merged.

@leaanthony
Copy link
Member

Thanks for the fix! Let's keep the PRs separate 👍

@leaanthony leaanthony merged commit 05eddc6 into wailsapp:master Sep 24, 2023
@leaanthony
Copy link
Member

Thanks for this! 🙏

@fkhadra
Copy link
Contributor Author

fkhadra commented Sep 25, 2023

Awesome! Ill open a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OSX] Allow to set tabFocusesLinks
2 participants