Skip to content

wayland: Clear preedit if there is no pending preedit.#2481

Merged
kchibisov merged 1 commit intorust-windowing:masterfrom
wengxt:master
Sep 9, 2022
Merged

wayland: Clear preedit if there is no pending preedit.#2481
kchibisov merged 1 commit intorust-windowing:masterfrom
wengxt:master

Conversation

@wengxt
Copy link
Copy Markdown
Contributor

@wengxt wengxt commented Sep 8, 2022

Fix #2478

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@wengxt wengxt requested a review from kchibisov as a code owner September 8, 2022 20:29
Copy link
Copy Markdown
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Thanks. Could you apply this, so we don't spend time reformatting it.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0e648fd5..9c39fcf4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 - On Wayland, a new `wayland-csd-adwaita-crossfont` feature was added to use `crossfont` instead of `ab_glyph` for decorations.
 - On Wayland, if not otherwise specified use upstream automatic CSD theme selection.
 - On Windows, fixed ALT+Space shortcut to open window menu.
+- On Wayland, fixed `Ime::Preedit` not being sent on IME reset.

 # 0.27.2 (2022-8-12)

diff --git a/src/platform_impl/linux/wayland/seat/text_input/handlers.rs b/src/platform_impl/linux/wayland/seat/text_input/handlers.rs
index 8f05bb60..f4f131d0 100644
--- a/src/platform_impl/linux/wayland/seat/text_input/handlers.rs
+++ b/src/platform_impl/linux/wayland/seat/text_input/handlers.rs
@@ -92,15 +92,21 @@ pub(super) fn handle_text_input(
                 event_sink.push_window_event(WindowEvent::Ime(Ime::Commit(text)), window_id);
             }

-            // Push preedit string we've got after latest commit.
-            if let Some(preedit) = inner.pending_preedit.take() {
-                let cursor_range = preedit
-                    .cursor_begin
-                    .map(|b| (b, preedit.cursor_end.unwrap_or(b)));
+            // Always send preedit on `Done` events.
+            let (text, range) = inner
+                .pending_preedit
+                .take()
+                .map(|preedit| {
+                    let cursor_range = preedit
+                        .cursor_begin
+                        .map(|b| (b, preedit.cursor_end.unwrap_or(b)));

-                let event = Ime::Preedit(preedit.text, cursor_range);
-                event_sink.push_window_event(WindowEvent::Ime(event), window_id);
-            }
+                    (preedit.text, cursor_range)
+                })
+                .unwrap_or_default();
+
+            let event = Ime::Preedit(text, range);
+            event_sink.push_window_event(WindowEvent::Ime(event), window_id);
         }
         _ => (),
     }

I think that should still work.

@kchibisov
Copy link
Copy Markdown
Member

Actually, I think I'd adjust all platforms to send empty preedit before the Commit. I'll keep this PR open for a while, but I can update it myself, I guess. Would need to adjust X11 handling here.

@wengxt wengxt requested a review from kchibisov September 8, 2022 22:06
@wengxt
Copy link
Copy Markdown
Contributor Author

wengxt commented Sep 8, 2022

applied, thanks. I

Thanks. Could you apply this, so we don't spend time reformatting it.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0e648fd5..9c39fcf4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 - On Wayland, a new `wayland-csd-adwaita-crossfont` feature was added to use `crossfont` instead of `ab_glyph` for decorations.
 - On Wayland, if not otherwise specified use upstream automatic CSD theme selection.
 - On Windows, fixed ALT+Space shortcut to open window menu.
+- On Wayland, fixed `Ime::Preedit` not being sent on IME reset.

 # 0.27.2 (2022-8-12)

diff --git a/src/platform_impl/linux/wayland/seat/text_input/handlers.rs b/src/platform_impl/linux/wayland/seat/text_input/handlers.rs
index 8f05bb60..f4f131d0 100644
--- a/src/platform_impl/linux/wayland/seat/text_input/handlers.rs
+++ b/src/platform_impl/linux/wayland/seat/text_input/handlers.rs
@@ -92,15 +92,21 @@ pub(super) fn handle_text_input(
                 event_sink.push_window_event(WindowEvent::Ime(Ime::Commit(text)), window_id);
             }

-            // Push preedit string we've got after latest commit.
-            if let Some(preedit) = inner.pending_preedit.take() {
-                let cursor_range = preedit
-                    .cursor_begin
-                    .map(|b| (b, preedit.cursor_end.unwrap_or(b)));
+            // Always send preedit on `Done` events.
+            let (text, range) = inner
+                .pending_preedit
+                .take()
+                .map(|preedit| {
+                    let cursor_range = preedit
+                        .cursor_begin
+                        .map(|b| (b, preedit.cursor_end.unwrap_or(b)));

-                let event = Ime::Preedit(preedit.text, cursor_range);
-                event_sink.push_window_event(WindowEvent::Ime(event), window_id);
-            }
+                    (preedit.text, cursor_range)
+                })
+                .unwrap_or_default();
+
+            let event = Ime::Preedit(text, range);
+            event_sink.push_window_event(WindowEvent::Ime(event), window_id);
         }
         _ => (),
     }

I think that should still work.

applied

@kchibisov kchibisov merged commit 92ddb34 into rust-windowing:master Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

text_input doesn't handle empty preedit correctly.

2 participants