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

TextEdit uses wrong copy/paste shortcuts on Mac in HTML5 build #68653

Closed
Ariorick opened this issue Nov 14, 2022 · 5 comments · Fixed by #75451
Closed

TextEdit uses wrong copy/paste shortcuts on Mac in HTML5 build #68653

Ariorick opened this issue Nov 14, 2022 · 5 comments · Fixed by #75451

Comments

@Ariorick
Copy link

Godot version

3.5.1 stable official

System information

macOS 12.1

Issue description

TextEdit uses wrong copy/paste shortcuts on Mac in htmpl5 build

Steps to reproduce

Create any projects with TextEdit, run as HTML on macOS. Right-click to see shortcuts being ctrl+C ctrl+v instead of using Command

Minimal reproduction project

No response

@akien-mga akien-mga changed the title TextEdit uses wrong copy/paste shortcuts on Mac in htmpl5 build TextEdit uses wrong copy/paste shortcuts on Mac in HTML5 build Nov 14, 2022
@akien-mga
Copy link
Member

CC @Faless @bruvzg

That's a tricky problem to solve because macOS/iOS-specific overrides for this kind of shortcut are defined at build time for the target platform (macOS and iOS native builds). The Web platform is cross-platform and thus doesn't use those overrides. We'd need significant refactoring to make the Apple-style hotkeys not a compile-time decision but something overridden at runtime, also taking into account the host platform for Web builds.

@bruvzg
Copy link
Member

bruvzg commented Nov 14, 2022

For the most part we are using ED_SHORTCUT_OVERRIDE which is internally using OS::has_feature("macos"), so it should be possible to detect host OS and add OS feature to the list. There are few #ifdef MACOS_ENABLED used, but it should be possible to replace them with runtime checks as well.

@bruvzg
Copy link
Member

bruvzg commented Nov 14, 2022

Completely untested mockup - https://github.com/bruvzg/godot/tree/web_mac_keys

@Faless
Copy link
Collaborator

Faless commented Nov 14, 2022

which is internally using OS::has_feature("macos"), so it should be possible to detect host OS and add OS feature to the list.

Won't that create issues in other code that use that feature thinking they are on OSX native while being in the web version? Maybe a dedicate feature is better?

@akien-mga
Copy link
Member

Yeah I think we might have to use a dedicated feature so that users can still perform different logic on macOS native and Web on macOS, e.g.:

if OS.has_feature("macos"):
    OS.execute("open", ["godot"])

should/can obviously not run on the Web.

Maybe we can define new features for each web+host combinations?

web_android
web_ios
web_linuxbsd
web_macos
web_windows

Then we could set ED_SHORTCUT_OVERRIDE for web_macos (and possibly web_ios, in case we ever get the editor ported to iOS and used with an Apple keyboard).

Duplicating all those overrides would be tedious however, so we can also special case it in ED_SHORTCUT_OVERRIDE:

diff --git a/editor/editor_settings.cpp b/editor/editor_settings.cpp
index bc186c7a16..20968f4729 100644
--- a/editor/editor_settings.cpp
+++ b/editor/editor_settings.cpp
@@ -1459,7 +1459,10 @@ void ED_SHORTCUT_OVERRIDE_ARRAY(const String &p_path, const String &p_feature, c
 
 	// Only add the override if the OS supports the provided feature.
 	if (!OS::get_singleton()->has_feature(p_feature)) {
-		return;
+		// Special case for "macos" overrides we want to use for Web on macOS/iOS.
+		if (!(p_feature == "macos" && (OS::get_singleton()->has_feature("web_macos") || OS::get_singleton()->has_feature("web_ios"))) {
+			return;
+		}
 	}
 
 	Array events;

That only covers the editor shortcuts - we'd also need to do something similar in InputMap::get_builtins_with_feature_overrides_applied I believe.

And finally, some more changes might be needed to handle the CMD_OR_CTRL remapping on web_macos and web_ios, for actions such as ui_copy:

        inputs = List<Ref<InputEvent>>();
        inputs.push_back(InputEventKey::create_reference(Key::C | KeyModifierMask::CMD_OR_CTRL));
        inputs.push_back(InputEventKey::create_reference(Key::INSERT | KeyModifierMask::CMD_OR_CTRL));
        default_builtin_cache.insert("ui_copy", inputs);

So that it uses and displays Cmd on those platforms.

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

Successfully merging a pull request may close this issue.

5 participants