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

Allow XOpenIM to return NULL? #277

Closed
liuw opened this issue Sep 3, 2017 · 13 comments · Fixed by #451
Closed

Allow XOpenIM to return NULL? #277

liuw opened this issue Sep 3, 2017 · 13 comments · Fixed by #451
Labels
DS - x11 S - enhancement Wouldn't this be the coolest?

Comments

@liuw
Copy link

liuw commented Sep 3, 2017

https://github.com/tomaka/winit/blob/3d1c18ded9f82c17e292c03e5740cb558800c30c/src/platform/linux/x11/mod.rs#L697

Winit will panic if XOpenIM returns NULL. It is too strict IMHO. As it stands, winit can't find a valid input method.

I would much prefer the way urxvt does it:

xim = XOpenIM (display->dpy, 0, 0, 0);

if (!xim)
    return false;

This is in relation to alacritty/alacritty/issues/732

(I'm in no way an expert on X11.)

@tomaka tomaka added DS - x11 S - enhancement Wouldn't this be the coolest? labels Sep 4, 2017
@Ralith
Copy link
Contributor

Ralith commented Sep 5, 2017

I don't know much about XIM but I expect urxvt probably has the right idea. It should be possible for all XIM code to be made optional without too much pain.

@alagopus
Copy link

Confirmed on elementary OS Loki 4.4.0-97-generic using 5ac42bb13bc68c5cbc44869dc9fc9ac19402a6e6 of https://github.com/jwilm/alacritty: starting ibus restores functionality. Without ibus, alacritty is defunct now.

@jwilm
Copy link
Contributor

jwilm commented Oct 18, 2017

For another reference, the terminal emulator st tries running XOpenIM with a few different locale modifiers:

/* input methods */
if ((xw.xim = XOpenIM(xw.dpy, NULL, NULL, NULL)) == NULL) {
	XSetLocaleModifiers("@im=local");
	if ((xw.xim =  XOpenIM(xw.dpy, NULL, NULL, NULL)) == NULL) {
		XSetLocaleModifiers("@im=");
		if ((xw.xim = XOpenIM(xw.dpy,
				NULL, NULL, NULL)) == NULL) {
			die("XOpenIM failed. Could not open input"
				" device.\n");
		}
	}
}

@Limeth
Copy link

Limeth commented Nov 26, 2017

@alagopus Weird, I am getting the error even when I try to run ibus-daemon in the background. Currently trying it with 8ff3c5d170abe47554f5d2a41ad35ddc451d254d of alacritty.

hcpl added a commit to hcpl/winit that referenced this issue Mar 19, 2018
@ariasuni
Copy link

ariasuni commented Mar 29, 2018

I get thread 'main' panicked at 'XOpenIM failed', /home/ariasuni/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.11.2/src/platform/linux/x11/mod.rs:959:17 when ibus is set, see my ~/.profile:

export GTK_IM_MODULE=ibus
export XMODIFIERS=@im=ibus
export QT_IM_MODULE=ibus

but is not launched for some reason.

Also, restarting ibus makes me unable to type text anymore (well, at least in alacritty, but I’m pretty sure it’s managed by winit). GTK apps works really well in that regard: they use ibus if it’s available, otherwise fallback to xim AND use ibus again as soon as it’s relaunched.

francesca64 pushed a commit to hcpl/winit that referenced this issue Mar 29, 2018
@francesca64
Copy link
Member

@ariasuni do you still experience that panic using #424?

I'm able to reproduce the issue with restarting ibus. In fact, it seems to cause the event loop to break entirely, and in my case, the program segfaults soon afterwards. I'll look into fixing it.

@ariasuni
Copy link

Sorry, how can I test that?

francesca64 pushed a commit to francesca64/winit that referenced this issue Mar 29, 2018
@francesca64
Copy link
Member

Assuming you're using Alacritty, you can add this to the Cargo.toml:

[patch.crates-io]
winit = { git = "https://github.com/hcpl/winit", branch = "xopenim-different-locales" }

@ariasuni
Copy link

It stills panic with the same message. :(

francesca64 pushed a commit to hcpl/winit that referenced this issue Apr 2, 2018
@francesca64
Copy link
Member

@ariasuni Could you please try this new branch:

[patch.crates-io]
winit = { git = "https://github.com/francesca64/winit", branch = "x11-xim-refresh" }

You'll also need to run cargo update afterwards.

It should fix both of the problems, and if it doesn't, I left some printlns in for now that will help me diagnose the issue.

@ariasuni
Copy link

ariasuni commented Apr 3, 2018

It works! Right now I still can’t type Japanese if I (re)start Ibus after the launch.

Here’s the console log:

IM Fallbacks(InputMethod { im: 0x7f62f0470380, name: "@im=local" })
(POTENTIAL PotentialInputMethods {
    xmodifiers: Some(
        PotentialInputMethod {
            name: "@im=ibus",
            failed: true
        }
    ),
    xim_servers: Some(
        [
            PotentialInputMethod {
                name: "@im=ibus",
                failed: true
            }
        ]
    ),
    fallbacks: [
        PotentialInputMethod {
            name: "@im=local",
            failed: false
        },
        PotentialInputMethod {
            name: "@im=",
            failed: false
        }
    ]
})

@francesca64
Copy link
Member

@ariasuni I've pushed some more changes, so cargo update and let me know if everything works fine now.

@ariasuni
Copy link

ariasuni commented Apr 4, 2018

Everything works as expected. This is awesome. ❤️

francesca64 pushed a commit to hcpl/winit that referenced this issue Apr 5, 2018
francesca64 pushed a commit that referenced this issue Apr 5, 2018
* Try XOpenIM with different locale modifiers

Implements the solution suggested in
#277 (comment).

* Use empty XSetLocaleModifiers beforehand

Also, for modifiers, convert from length-based UTF-8 strings to
null-terminated bytestrings.

* Add CHANGELOG entry and comments
francesca64 added a commit to francesca64/winit that referenced this issue Apr 7, 2018
Fixes rust-windowing#195
Fixes rust-windowing#277

* Read `XMODIFIERS` explicitly/directly instead of calling `XSetLocaleModifiers` with an
empty string. This is useful for debugging purposes, and more clear to read and handle.
* Fallback to local input method if the one specified in `XMODIFIERS` is later closed on the
server end (i.e. if ibus/fcitx is terminated). Previously, that would cause the event loop
to freeze and usually also segfault.
* If using the fallback input method, respond to the `XMODIFIERS` input method later
becoming available. This means that the input method restarting is handled, and that even if
the program was started while ibus/fcitx/etc. was unavailable, it will start using it as
soon as it becomes available.
* Only one input method is opened for the whole event loop, with each window having its own
input context.
* IME works completely out of the box now, no longer requiring application developers to
call `setlocale` or `XSetLocaleModifiers`.
* Detailed error messages are provided if no input method could be opened. However, no
information is provided to the user if their intended `XMODIFIERS` input method failed to
open but the fallbacks (which will ostensibly always succeed) succeeded; in my opinion, this
is something that is best filled by adding a logging feature to winit.
francesca64 added a commit that referenced this issue Apr 11, 2018
Fixes #195
Fixes #277
Fixes #455

* Read `XMODIFIERS` explicitly/directly instead of calling `XSetLocaleModifiers` with an
empty string. This is useful for debugging purposes, and more clear to read and handle.
* Fallback to local input method if the one specified in `XMODIFIERS` is later closed on the
server end (i.e. if ibus/fcitx is terminated). Previously, that would cause the event loop
to freeze and usually also segfault.
* If using the fallback input method, respond to the `XMODIFIERS` input method later
becoming available. This means that the input method restarting is handled, and that even if
the program was started while ibus/fcitx/etc. was unavailable, it will start using it as
soon as it becomes available.
* Only one input method is opened for the whole event loop, with each window having its own
input context.
* IME works completely out of the box now, no longer requiring application developers to
call `setlocale` or `XSetLocaleModifiers`.
* Detailed error messages are provided if no input method could be opened. However, no
information is provided to the user if their intended `XMODIFIERS` input method failed to
open but the fallbacks (which will ostensibly always succeed) succeeded; in my opinion, this
is something that is best filled by adding a logging feature to winit.
tmfink pushed a commit to tmfink/winit that referenced this issue Jan 5, 2022
madsmtm added a commit to madsmtm/winit that referenced this issue Jun 11, 2022
* refactor(windows): `begin_resize_drag` now similar to gtk's (rust-windowing#200)

* refactor(windows): `begin_resize_drag` now similart to gtk's

* fix

* feat(linux): skipping taskbar will now also skip pager (rust-windowing#198)

* refactor(linux): clean dummy device_id (rust-windowing#195)

* refactor(linux): clean dummy device_id

* fmt

* feat(linux): allow resizing undecorated window using touch (rust-windowing#199)

* refactor(windows): only skip taskbar if needed when `set_visible` is called (rust-windowing#196)

* fix: increase borderless resizing inset (rust-windowing#202)

* fix: increase borderless resizing inset

* update some comments

* Replace winapi with windows crate bindings shared with WRY (rust-windowing#206)

* fix(deps): update rust crate libayatana-appindicator to 0.1.6 (rust-windowing#190)

Co-authored-by: Renovate Bot <[email protected]>

* Add Windows crate and webview2-com-sys bindings

* Initial port to webview2-com-sys

* Finish conversion and remove winapi

* Fix renamed lint warning

* Fix all match arms referencing const variables

* Put back the assert instead of expect

* Point to the published version of webview2-com-sys

* Cleanup slightly weird BOOL handling

* Replace mem::zeroed with Default::default

* Add a summary in .changes

* Remove extra projects not in config.json

* Fix clippy warnings

* Update to 32-bit compatible webview2-com-sys

* Better fix for merge conflict

* Fix clippy errors on Windows

* Use path prefix to prevent variable shadowing

* Fix Windows clippy warnings with nightly toolchain

* Fix Linux nightly/stable clippy warnings

* Fix macOS nightly/stable clippy warnings

* Put back public *mut libc::c_void for consistency

* Re-run cargo fmt

* Move call_default_window_proc to util mod

* Remove unnecessary util::to_wstring calls

* Don't repeat LRESULT expression in match arms

* Replace bitwise operations with util functions

* Cleanup more bit mask & shift with util fns

* Prefer from conversions instead of as cast

* Implement get_xbutton_wparam

* Use *mut libc::c_void for return types

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>

* fix(keyboard): add mapping for space key on Windows (rust-windowing#209)

* fix(keyboard): add mapping for space key on Windows

* change file

* feat: impl Clone for EventLoopWindowTarget (rust-windowing#211)

* chore: add `on_issue_closed.yml` (rust-windowing#214)

* Update tray dependency version (rust-windowing#217)

* Delete on_issue_closed.yml (rust-windowing#221)

* refactor(linux): event loop (rust-windowing#233)

* Use crossbeam::channel

* Fix crossbeam channel import

* Add check on poll event

* Fix deadlock when unregistering shortcut on Linux (rust-windowing#230)

* Add fullscreen monitor selection support on Linux (rust-windowing#235)

* Add fullscreen monitor support on Linux

* Add change file

* Remove todo on videomode

* Fix clippy

* Update to 2021 edition (rust-windowing#236)

* Update to 2021 edition

* Fix clippy

* Add run_return on Linux (rust-windowing#237)

* Add run_return on Linux

* Add main context

* Add run_return trait on Linux (rust-windowing#238)

* Fix: rust-windowing#239 Update webview2-com and windows crates (rust-windowing#240)

* Replace webivew2-com-sys with prebuilt windows

* Use windows utility instead of direct GetLastError

* Bump windows version and add changelog

* Run cargo fmt

* Restore inverted matches macro

* Scope constants in match arms

* Fix inverted null check

* Update src/platform_impl/windows/util.rs

Co-authored-by: Amr Bashir <[email protected]>

* Use env_logger instead of simple_logger (rust-windowing#241)

* Use env_logger instead of simple_logger

* Make clippy happy

* Cherry pick commits to next (rust-windowing#244)

* feat(macos): Add `unhide_application` method, closes rust-windowing#182 (rust-windowing#231)

* feat(macos): Add `unhide_application` method

* Update src/platform/macos.rs

Co-authored-by: Amr Bashir <[email protected]>

* Reanme to `show_application()`

* Remove broken doc link

Co-authored-by: Amr Bashir <[email protected]>

* feat: Allow more strings to parse to keycode (rust-windowing#229)

* feat: support accelerator key strings `,` `-` `.` `Space` `Tab` and `F13`-`F24` (rust-windowing#228)

* feat(macOS): support more accelerator key strings

* Move function keys together

* Add `,` `-` `.` `Space` `F20-F24` for Windows

* Remove support for accelerators not found in `winapi`

* Add `,` `-` `.` `Space` `F13-F24` for Linux

* Update .changes

* Add the rest for Windows

* Add the rest for Linux

* Add the rest on macOS

* Update accelerator-strings.md

* Fix git comments

Co-authored-by: Kasper <[email protected]>
Co-authored-by: Amr Bashir <[email protected]>

* Add redraw events on Linux (rust-windowing#245)

* Add redraw events on Linux

* Update doc of RequestRedraw Event

* Add change file

* Fix missing menu bar on borderless window (rust-windowing#247)

Credit goes to irh's work on winit commit f2de847

* refactor: improve `set_skip_taskbar` impl on Windows (rust-windowing#250)

* fix: emit errors on parsing an invalid accelerator for string, closes rust-windowing#135 (rust-windowing#252)

* chore: update comment

* fix(linux): fix focus events not firing properly (rust-windowing#253)

* fix(linux): fix focus events not firing properly

* add changelog

* chore: update focus events error message

* chore: fmt

* fix: revert windows-rs 0.28 version bump

* fix(linux): fix native menu items (rust-windowing#256)

* chore: remove examples commited by accident

* Update `ReceivedImeText` (rust-windowing#251)

* Allow receiving text without Ime on Windows

* Avoid panic todo

* Receive text without ime on mac

* Fix CursorMoved event on Linux

* Add ReceivedImeText on Linux

This only add Simple IME from GTK for now. We should add the actual IME
from system in the future.

* Fix redraw event that causes inifinite loop (rust-windowing#260)

* Fix redraw event that causes inifinite loop

* Refactor event loop

* Remove unused function

* Add doc comment on linux's run_return

* Ignore doc test on run_return

* Add non blocking iteration on Linux (rust-windowing#261)

* Docs: SystemTrayExtWindows::remove() is gone (rust-windowing#262)

Fix docs following rust-windowing#153

* Fix busy loop on Linux (rust-windowing#265)

* Update windows crate to 0.29.0 (rust-windowing#266)

* Update to windows 0.29.0

* Add change description

* Remove clippy check (rust-windowing#267)

* refactor(windows): align util function with win32 names

* chore: update PR template

* fix(linux): fire resized & moved events on min/maximize, closes rust-windowing#219 (rust-windowing#254)

* feat(linux): implement `raw_window_handle()` (rust-windowing#269)

* chore(deps): update to raw-window-handle 0.4

* add linux raw-window-handle support

* update macos/ios/android

* fix ios

* Fix core-video-sys dependency (rust-windowing#274)

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

* Fix some invalid msg_send! usage (rust-windowing#276)

* Revert "Fix some invalid msg_send! usage (rust-windowing#276)" (rust-windowing#277)

This reverts commit a3a2e0cfc49ddfa8cdf65cf9870fb8e3d45b4bc0.

* Revert "The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)" (rust-windowing#279)

This reverts commit 6f9c468f26ddb60e29be2139397bfaf3b30eab1e.

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#280)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

Co-authored-by: madsmtm <[email protected]>

* Fix some invalid msg_send! usage (rust-windowing#278)

Co-authored-by: madsmtm <[email protected]>

* Add exit code to ControlFlow::Exit (rust-windowing#281)

* Add exit code to ControlFlow::Exit

* Cargo fmt

* Add change files

Co-authored-by:  multisn8 <[email protected]>

* Add new_any_thread to Unix event loop (rust-windowing#282)

* Update windows crate to 0.30.0 (rust-windowing#283)

* Update windows crate to 0.30.0

* Simplify new-type usage

* Fix boxing in GWL_USERDATA

* Make sure everyone is using Get/SetWindowLongPtrW

* build the system_tray module when "ayatana" feature is enabled (rust-windowing#285)

Without those cfg feature checks, the "ayatana" feature does
actually not enable anything.

* Fix click events missing whe tray has menu (rust-windowing#291)

* Fix click events missing whe tray has menu

* Add change file

* Fix crash when tray has no menu (rust-windowing#294)

* chore: update pull request commit exmple

* fix(windows): send correct position for system tray events, closes rust-windowing#295 (rust-windowing#300)

* fix(windows): revert maximized state handling to winit impl, closes rust-windowing#193 (rust-windowing#299)

* fix(windows): revet maximized state handling to winit impl, closes rust-windowing#193

* add chanefile [skip ci]

* fix: `MenuItem::Quit` on Windows (rust-windowing#303)

* fix: `MenuItem::Close` on Windows

* use `PostQuitMessage` instead

Co-authored-by: amrbashir <[email protected]>

* feat: v1 audit by Radically Open Security (rust-windowing#304)

* Update to gtk 0.15 (rust-windowing#288)

* Update to gtk 0.15

* Fix picky none on set_geometry_hint

* Fix CursorMoved position

Co-authored-by: Amr Bashir <[email protected]>
Co-authored-by: Bill Avery <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
Co-authored-by: Kasper <[email protected]>
Co-authored-by: amrbashir <[email protected]>
Co-authored-by: Jay Oster <[email protected]>
Co-authored-by: madsmtm <[email protected]>
Co-authored-by: multisn8 <[email protected]>
Co-authored-by: Aurélien Jacobs <[email protected]>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DS - x11 S - enhancement Wouldn't this be the coolest?
Development

Successfully merging a pull request may close this issue.

8 participants