From f44b8dde2103018543b787ff6f00b2943deb7aac Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Thu, 28 Nov 2024 11:59:10 +0100
Subject: [PATCH 01/14] Initial IME support

---
 masonry/src/text/editor.rs      | 94 ++++++++++++++++++++++++++++++---
 masonry/src/widget/text_area.rs | 71 ++++++++++++++++++++++---
 2 files changed, 152 insertions(+), 13 deletions(-)

diff --git a/masonry/src/text/editor.rs b/masonry/src/text/editor.rs
index 62f94e7dd..55c443b4d 100644
--- a/masonry/src/text/editor.rs
+++ b/masonry/src/text/editor.rs
@@ -5,7 +5,7 @@
 
 //! Import of Parley's `PlainEditor` as the version in Parley is insufficient for our needs.
 
-use core::{cmp::PartialEq, default::Default, fmt::Debug};
+use core::{cmp::PartialEq, default::Default, fmt::Debug, ops::Range};
 
 use accesskit::{Node, NodeId, TreeUpdate};
 use parley::layout::LayoutAccessibility;
@@ -49,6 +49,9 @@ where
     layout: Layout<T>,
     layout_access: LayoutAccessibility,
     selection: Selection,
+    /// Byte offsets of IME composing preedit text in the text buffer.
+    /// `None` if the IME is not currently composing.
+    compose: Option<Range<usize>>,
     width: Option<f32>,
     scale: f32,
     // Simple tracking of when the layout needs to be updated
@@ -76,6 +79,7 @@ where
             layout: Default::default(),
             layout_access: Default::default(),
             selection: Default::default(),
+            compose: None,
             width: None,
             scale: 1.0,
             layout_dirty: true,
@@ -214,6 +218,70 @@ where
         }
     }
 
+    // --- MARK: IME ---
+    /// Set the IME preedit composing text.
+    ///
+    /// This starts composing. Composing is reset by calling [`PlainEditorTxn::clear_compose`].
+    /// While composing, it is a logic error to call anything other than
+    /// [`PlainEditorTxn::set_compose`] or [`PlainEditorTxn::clear_compose`].
+    ///
+    /// The preedit text replaces the current selection if this call starts composing.
+    ///
+    /// The selection is updated based on `cursor`, which contains the byte offsets relative to the
+    /// start of the preedit text. If `cursor` is `None`, the selection is collapsed to a caret in
+    /// front of the preedit text.
+    pub fn set_compose(&mut self, text: &str, cursor: Option<(usize, usize)>) {
+        debug_assert!(!text.is_empty());
+        debug_assert!(cursor.map(|cursor| cursor.1 <= text.len()).unwrap_or(true));
+
+        let start = if let Some(preedit_range) = self.editor.compose.clone() {
+            self.editor
+                .buffer
+                .replace_range(preedit_range.clone(), text);
+            preedit_range.start
+        } else {
+            if self.editor.selection.is_collapsed() {
+                self.editor
+                    .buffer
+                    .insert_str(self.editor.selection.text_range().start, text);
+            } else {
+                self.editor
+                    .buffer
+                    .replace_range(self.editor.selection.text_range(), text);
+            }
+            self.editor.selection.text_range().start
+        };
+        self.editor.compose = Some(start..start + text.len());
+        self.update_layout();
+
+        if let Some(cursor) = cursor {
+            // Select the location indicated by the IME.
+            self.editor.set_selection(Selection::new(
+                self.editor.cursor_at(start + cursor.0),
+                self.editor.cursor_at(start + cursor.1),
+            ));
+        } else {
+            // IME indicates nothing is to be selected: collapse the selection to a
+            // caret just in front of the preedit.
+            self.editor
+                .set_selection(self.editor.cursor_at(start).into());
+        }
+    }
+
+    /// Stop IME composing.
+    ///
+    /// This removes the IME preedit text.
+    pub fn clear_compose(&mut self) {
+        if let Some(preedit_range) = self.editor.compose.clone() {
+            self.editor.buffer.replace_range(preedit_range.clone(), "");
+            self.editor.compose = None;
+            self.update_layout();
+
+            self.editor
+                .set_selection(self.editor.cursor_at(preedit_range.start).into());
+        }
+    }
+
     // --- MARK: Cursor Movement ---
     /// Move the cursor to the cluster boundary nearest this point in the layout.
     pub fn move_to_point(&mut self, x: f32, y: f32) {
@@ -513,12 +581,8 @@ where
     fn cursor_at(&self, index: usize) -> Cursor {
         // TODO: Do we need to be non-dirty?
         // FIXME: `Selection` should make this easier
-        if index >= self.buffer.len() {
-            Cursor::from_byte_index(
-                &self.layout,
-                self.buffer.len().saturating_sub(1),
-                Affinity::Upstream,
-            )
+        if index > self.buffer.len() {
+            Cursor::from_byte_index(&self.layout, self.buffer.len(), Affinity::Upstream)
         } else {
             Cursor::from_byte_index(&self.layout, index, Affinity::Downstream)
         }
@@ -667,6 +731,17 @@ where
         for prop in self.default_style.inner().values() {
             builder.push_default(prop.to_owned());
         }
+        if let Some(ref preedit_range) = self.compose {
+            // TODO: underline currently doesn't show up, maybe the brush is invisible?
+            builder.push(
+                parley::style::StyleProperty::UnderlineBrush(Some(T::default())),
+                preedit_range.clone(),
+            );
+            builder.push(
+                parley::style::StyleProperty::Underline(true),
+                preedit_range.clone(),
+            );
+        }
         self.layout = builder.build(&self.buffer);
         self.layout.break_all_lines(self.width);
         self.layout.align(self.width, self.alignment);
@@ -675,6 +750,11 @@ where
         self.generation.nudge();
     }
 
+    /// Whether the editor is currently in IME composing mode.
+    pub fn is_composing(&self) -> bool {
+        self.compose.is_some()
+    }
+
     pub fn accessibility(
         &mut self,
         update: &mut TreeUpdate,
diff --git a/masonry/src/widget/text_area.rs b/masonry/src/widget/text_area.rs
index 657bc1751..96ca53766 100644
--- a/masonry/src/widget/text_area.rs
+++ b/masonry/src/widget/text_area.rs
@@ -448,6 +448,10 @@ impl<const EDITABLE: bool> TextArea<EDITABLE> {
 // --- MARK: IMPL WIDGET ---
 impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
     fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
+        if self.editor.is_composing() {
+            return;
+        }
+
         let window_origin = ctx.widget_state.window_origin();
         let (fctx, lctx) = ctx.text_contexts();
         let is_rtl = self.editor.layout(fctx, lctx).is_rtl();
@@ -508,7 +512,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
     fn on_text_event(&mut self, ctx: &mut EventCtx, event: &TextEvent) {
         match event {
             TextEvent::KeyboardKey(key_event, modifiers_state) => {
-                if !key_event.state.is_pressed() {
+                if !key_event.state.is_pressed() || self.editor.is_composing() {
                     return;
                 }
                 #[allow(unused)]
@@ -709,7 +713,50 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
             TextEvent::FocusChange(_) => {}
             TextEvent::Ime(e) => {
                 // TODO: Handle the cursor movement things from https://github.com/rust-windowing/winit/pull/3824
-                tracing::warn!(event = ?e, "TextArea doesn't accept IME");
+                let (fctx, lctx) = ctx.text_contexts();
+                // The text to submit as the TextChanged action. We do not send a TextChange action
+                // for the "virtual" preedit text.
+                let mut submit_text = None;
+                match e {
+                    winit::event::Ime::Disabled => {
+                        self.editor.transact(fctx, lctx, |txn| txn.clear_compose());
+                    }
+                    winit::event::Ime::Preedit(text, cursor) => {
+                        if text.is_empty() {
+                            self.editor.transact(fctx, lctx, |txn| txn.clear_compose());
+                        } else {
+                            if !self.editor.is_composing() && self.editor.selected_text().is_some()
+                            {
+                                // The IME has started composing. Delete the current selection and
+                                // send a TextChange event with the selection removed, but without
+                                // the composing preedit text.
+                                self.editor.transact(fctx, lctx, |txn| {
+                                    txn.delete_selection();
+                                });
+                                submit_text = Some(self.text().to_string());
+                            }
+
+                            self.editor
+                                .transact(fctx, lctx, |txn| txn.set_compose(text, *cursor));
+                        }
+                    }
+                    winit::event::Ime::Commit(text) => {
+                        self.editor
+                            .transact(fctx, lctx, |txn| txn.insert_or_replace_selection(text));
+                        submit_text = Some(self.text().to_string());
+                    }
+                    _ => {}
+                }
+
+                ctx.set_handled();
+                if let Some(text) = submit_text {
+                    ctx.submit_action(dbg!(crate::Action::TextChanged(text)));
+                }
+                let new_generation = self.editor.generation();
+                if new_generation != self.rendered_generation {
+                    ctx.request_layout();
+                    self.rendered_generation = new_generation;
+                }
             }
             TextEvent::ModifierChange(_) => {}
         }
@@ -720,12 +767,15 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
     }
 
     fn accepts_text_input(&self) -> bool {
-        // TODO: Implement IME, then flip back to EDITABLE.
-        false
+        EDITABLE
     }
 
     fn on_access_event(&mut self, ctx: &mut EventCtx, event: &AccessEvent) {
         if event.action == accesskit::Action::SetTextSelection {
+            if self.editor.is_composing() {
+                return;
+            }
+
             if let Some(accesskit::ActionData::SetTextSelection(selection)) = &event.data {
                 let (fctx, lctx) = ctx.text_contexts();
                 self.editor
@@ -738,8 +788,17 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
 
     fn update(&mut self, ctx: &mut UpdateCtx, event: &Update) {
         match event {
-            Update::FocusChanged(_) => {
-                ctx.request_render();
+            Update::FocusChanged(focused) => {
+                // HACK: currently, when moving focus away from a text area, the Ime::Disabled
+                // event is routed to the newly focused widget. Do IME clean up here.
+                if !focused && self.editor.is_composing() {
+                    let (fctx, lctx) = ctx.text_contexts();
+                    self.editor.transact(fctx, lctx, |txn| txn.clear_compose());
+                    self.rendered_generation = self.editor.generation();
+                    ctx.request_layout();
+                } else {
+                    ctx.request_render();
+                }
             }
             Update::DisabledChanged(_) => {
                 // We might need to use the disabled brush, and stop displaying the selection.

From 03aa8298c736fc374f20d04a8520db992e1b18ee Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Thu, 28 Nov 2024 13:43:40 +0100
Subject: [PATCH 02/14] Remove dbg!()

---
 masonry/src/widget/text_area.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/masonry/src/widget/text_area.rs b/masonry/src/widget/text_area.rs
index 96ca53766..cae2f6176 100644
--- a/masonry/src/widget/text_area.rs
+++ b/masonry/src/widget/text_area.rs
@@ -750,7 +750,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
 
                 ctx.set_handled();
                 if let Some(text) = submit_text {
-                    ctx.submit_action(dbg!(crate::Action::TextChanged(text)));
+                    ctx.submit_action(crate::Action::TextChanged(text));
                 }
                 let new_generation = self.editor.generation();
                 if new_generation != self.rendered_generation {

From 9bb6fb38876d4c4381f19a294b97683e8f33385c Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Thu, 28 Nov 2024 15:59:03 +0100
Subject: [PATCH 03/14] Debug-assert not composing

---
 masonry/src/text/editor.rs | 72 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/masonry/src/text/editor.rs b/masonry/src/text/editor.rs
index 55c443b4d..51ccda73c 100644
--- a/masonry/src/text/editor.rs
+++ b/masonry/src/text/editor.rs
@@ -110,17 +110,23 @@ where
     // --- MARK: Forced relayout ---
     /// Insert at cursor, or replace selection.
     pub fn insert_or_replace_selection(&mut self, s: &str) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .replace_selection(self.font_cx, self.layout_cx, s);
     }
 
     /// Delete the selection.
     pub fn delete_selection(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.insert_or_replace_selection("");
     }
 
     /// Delete the selection or the next cluster (typical ‘delete’ behavior).
     pub fn delete(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         if self.editor.selection.is_collapsed() {
             // Upstream cluster range
             if let Some(range) = self
@@ -142,6 +148,8 @@ where
 
     /// Delete the selection or up to the next word boundary (typical ‘ctrl + delete’ behavior).
     pub fn delete_word(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         if self.editor.selection.is_collapsed() {
             let focus = self.editor.selection.focus();
             let start = focus.index();
@@ -161,6 +169,8 @@ where
 
     /// Delete the selection or the previous cluster (typical ‘backspace’ behavior).
     pub fn backdelete(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         if self.editor.selection.is_collapsed() {
             // Upstream cluster
             if let Some(cluster) = self
@@ -201,6 +211,8 @@ where
 
     /// Delete the selection or back to the previous word boundary (typical ‘ctrl + backspace’ behavior).
     pub fn backdelete_word(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         if self.editor.selection.is_collapsed() {
             let focus = self.editor.selection.focus();
             let end = focus.index();
@@ -285,6 +297,8 @@ where
     // --- MARK: Cursor Movement ---
     /// Move the cursor to the cluster boundary nearest this point in the layout.
     pub fn move_to_point(&mut self, x: f32, y: f32) {
+        debug_assert!(!self.editor.is_composing());
+
         self.refresh_layout();
         self.editor
             .set_selection(Selection::from_point(&self.editor.layout, x, y));
@@ -294,6 +308,8 @@ where
     ///
     /// No-op if index is not a char boundary.
     pub fn move_to_byte(&mut self, index: usize) {
+        debug_assert!(!self.editor.is_composing());
+
         if self.editor.buffer.is_char_boundary(index) {
             self.refresh_layout();
             self.editor
@@ -303,6 +319,8 @@ where
 
     /// Move the cursor to the start of the buffer.
     pub fn move_to_text_start(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(self.editor.selection.move_lines(
             &self.editor.layout,
             isize::MIN,
@@ -312,12 +330,16 @@ where
 
     /// Move the cursor to the start of the physical line.
     pub fn move_to_line_start(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .set_selection(self.editor.selection.line_start(&self.editor.layout, false));
     }
 
     /// Move the cursor to the end of the buffer.
     pub fn move_to_text_end(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(self.editor.selection.move_lines(
             &self.editor.layout,
             isize::MAX,
@@ -327,12 +349,16 @@ where
 
     /// Move the cursor to the end of the physical line.
     pub fn move_to_line_end(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .set_selection(self.editor.selection.line_end(&self.editor.layout, false));
     }
 
     /// Move up to the closest physical cluster boundary on the previous line, preserving the horizontal position for repeated movements.
     pub fn move_up(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -342,12 +368,16 @@ where
 
     /// Move down to the closest physical cluster boundary on the next line, preserving the horizontal position for repeated movements.
     pub fn move_down(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .set_selection(self.editor.selection.next_line(&self.editor.layout, false));
     }
 
     /// Move to the next cluster left in visual order.
     pub fn move_left(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -357,6 +387,8 @@ where
 
     /// Move to the next cluster right in visual order.
     pub fn move_right(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -366,6 +398,8 @@ where
 
     /// Move to the next word boundary left.
     pub fn move_word_left(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -375,6 +409,8 @@ where
 
     /// Move to the next word boundary right.
     pub fn move_word_right(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -384,6 +420,8 @@ where
 
     /// Select the whole buffer.
     pub fn select_all(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             Selection::from_byte_index(&self.editor.layout, 0_usize, Affinity::default())
                 .move_lines(&self.editor.layout, isize::MAX, true),
@@ -392,11 +430,15 @@ where
 
     /// Collapse selection into caret.
     pub fn collapse_selection(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(self.editor.selection.collapse());
     }
 
     /// Move the selection focus point to the start of the buffer.
     pub fn select_to_text_start(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(self.editor.selection.move_lines(
             &self.editor.layout,
             isize::MIN,
@@ -406,12 +448,16 @@ where
 
     /// Move the selection focus point to the start of the physical line.
     pub fn select_to_line_start(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .set_selection(self.editor.selection.line_start(&self.editor.layout, true));
     }
 
     /// Move the selection focus point to the end of the buffer.
     pub fn select_to_text_end(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(self.editor.selection.move_lines(
             &self.editor.layout,
             isize::MAX,
@@ -421,12 +467,16 @@ where
 
     /// Move the selection focus point to the end of the physical line.
     pub fn select_to_line_end(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .set_selection(self.editor.selection.line_end(&self.editor.layout, true));
     }
 
     /// Move the selection focus point up to the nearest cluster boundary on the previous line, preserving the horizontal position for repeated movements.
     pub fn select_up(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -436,12 +486,16 @@ where
 
     /// Move the selection focus point down to the nearest cluster boundary on the next line, preserving the horizontal position for repeated movements.
     pub fn select_down(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .set_selection(self.editor.selection.next_line(&self.editor.layout, true));
     }
 
     /// Move the selection focus point to the next cluster left in visual order.
     pub fn select_left(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -451,12 +505,16 @@ where
 
     /// Move the selection focus point to the next cluster right in visual order.
     pub fn select_right(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor
             .set_selection(self.editor.selection.next_visual(&self.editor.layout, true));
     }
 
     /// Move the selection focus point to the next word boundary left.
     pub fn select_word_left(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -466,6 +524,8 @@ where
 
     /// Move the selection focus point to the next word boundary right.
     pub fn select_word_right(&mut self) {
+        debug_assert!(!self.editor.is_composing());
+
         self.editor.set_selection(
             self.editor
                 .selection
@@ -475,6 +535,8 @@ where
 
     /// Select the word at the point.
     pub fn select_word_at_point(&mut self, x: f32, y: f32) {
+        debug_assert!(!self.editor.is_composing());
+
         self.refresh_layout();
         self.editor
             .set_selection(Selection::word_from_point(&self.editor.layout, x, y));
@@ -482,6 +544,8 @@ where
 
     /// Select the physical line at the point.
     pub fn select_line_at_point(&mut self, x: f32, y: f32) {
+        debug_assert!(!self.editor.is_composing());
+
         self.refresh_layout();
         let line = Selection::line_from_point(&self.editor.layout, x, y);
         self.editor.set_selection(line);
@@ -489,6 +553,8 @@ where
 
     /// Move the selection focus point to the cluster boundary closest to point.
     pub fn extend_selection_to_point(&mut self, x: f32, y: f32) {
+        debug_assert!(!self.editor.is_composing());
+
         self.refresh_layout();
         // FIXME: This is usually the wrong way to handle selection extension for mouse moves, but not a regression.
         self.editor.set_selection(
@@ -502,6 +568,8 @@ where
     ///
     /// No-op if index is not a char boundary.
     pub fn extend_selection_to_byte(&mut self, index: usize) {
+        debug_assert!(!self.editor.is_composing());
+
         if self.editor.buffer.is_char_boundary(index) {
             self.refresh_layout();
             self.editor
@@ -513,6 +581,8 @@ where
     ///
     /// No-op if either index is not a char boundary.
     pub fn select_byte_range(&mut self, start: usize, end: usize) {
+        debug_assert!(!self.editor.is_composing());
+
         if self.editor.buffer.is_char_boundary(start) && self.editor.buffer.is_char_boundary(end) {
             self.refresh_layout();
             self.editor.set_selection(Selection::new(
@@ -523,6 +593,8 @@ where
     }
 
     pub fn select_from_accesskit(&mut self, selection: &accesskit::TextSelection) {
+        debug_assert!(!self.editor.is_composing());
+
         self.refresh_layout();
         if let Some(selection) = Selection::from_access_selection(
             selection,

From 260c52353ea3db6c5ce0c12f5330dd1182ea3752 Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Thu, 28 Nov 2024 16:07:15 +0100
Subject: [PATCH 04/14] Fix off-by-one (selecting last probably should have
 upstream affinity)

---
 masonry/src/text/editor.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/masonry/src/text/editor.rs b/masonry/src/text/editor.rs
index 51ccda73c..f348740ea 100644
--- a/masonry/src/text/editor.rs
+++ b/masonry/src/text/editor.rs
@@ -653,7 +653,7 @@ where
     fn cursor_at(&self, index: usize) -> Cursor {
         // TODO: Do we need to be non-dirty?
         // FIXME: `Selection` should make this easier
-        if index > self.buffer.len() {
+        if index >= self.buffer.len() {
             Cursor::from_byte_index(&self.layout, self.buffer.len(), Affinity::Upstream)
         } else {
             Cursor::from_byte_index(&self.layout, index, Affinity::Downstream)

From 5c811c60fefca5373f89c2e8ba4917de741c8951 Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 29 Nov 2024 10:56:32 +0100
Subject: [PATCH 05/14] Underline works again

---
 masonry/src/text/editor.rs | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/masonry/src/text/editor.rs b/masonry/src/text/editor.rs
index f348740ea..eb0c33dc6 100644
--- a/masonry/src/text/editor.rs
+++ b/masonry/src/text/editor.rs
@@ -804,11 +804,6 @@ where
             builder.push_default(prop.to_owned());
         }
         if let Some(ref preedit_range) = self.compose {
-            // TODO: underline currently doesn't show up, maybe the brush is invisible?
-            builder.push(
-                parley::style::StyleProperty::UnderlineBrush(Some(T::default())),
-                preedit_range.clone(),
-            );
             builder.push(
                 parley::style::StyleProperty::Underline(true),
                 preedit_range.clone(),

From c2adbcf01b899022f0154bca1b820b7310b72fe8 Mon Sep 17 00:00:00 2001
From: Tom Churchman <thomas@kepow.org>
Date: Fri, 29 Nov 2024 14:16:07 +0100
Subject: [PATCH 06/14] Update masonry/src/widget/text_area.rs

Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
---
 masonry/src/widget/text_area.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/masonry/src/widget/text_area.rs b/masonry/src/widget/text_area.rs
index cae2f6176..9a61c4584 100644
--- a/masonry/src/widget/text_area.rs
+++ b/masonry/src/widget/text_area.rs
@@ -745,7 +745,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
                             .transact(fctx, lctx, |txn| txn.insert_or_replace_selection(text));
                         submit_text = Some(self.text().to_string());
                     }
-                    _ => {}
+                    winit::event::Ime::Enabled => {}
                 }
 
                 ctx.set_handled();

From 98ce9e49adf465f6272d1c8dbac4f1df191afad3 Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 29 Nov 2024 14:41:53 +0100
Subject: [PATCH 07/14] Synthesize Ime::Disabled in focus update pass

---
 masonry/src/passes/update.rs    | 11 ++++++++++-
 masonry/src/widget/text_area.rs | 13 ++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index fd76e56e5..c341e8852 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -11,9 +11,12 @@ use crate::passes::{enter_span, enter_span_if, merge_state_up, recurse_on_childr
 use crate::render_root::{RenderRoot, RenderRootSignal, RenderRootState};
 use crate::tree_arena::ArenaMut;
 use crate::{
-    PointerEvent, QueryCtx, RegisterCtx, Update, UpdateCtx, Widget, WidgetId, WidgetState,
+    PointerEvent, QueryCtx, RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
+    WidgetState,
 };
 
+use super::event::run_on_text_event_pass;
+
 // --- MARK: HELPERS ---
 fn get_id_path(root: &RenderRoot, widget_id: Option<WidgetId>) -> Vec<WidgetId> {
     let Some(widget_id) = widget_id else {
@@ -471,6 +474,12 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         // We also request accessibility, because build_access_node() depends on the focus state.
         if let Some(prev_focused) = prev_focused {
             if root.widget_arena.has(prev_focused) {
+                if was_ime_active {
+                    // IME was active, but the next focused widget is going to receive the
+                    // Ime::Disabled event. Synthesize an Ime::Disabled event here and send it to
+                    // the widget about to be unfocused.
+                    run_on_text_event_pass(root, &TextEvent::Ime(winit::event::Ime::Disabled));
+                }
                 run_single_update_pass(root, prev_focused, |widget, ctx| {
                     widget.update(ctx, &Update::FocusChanged(false));
                     ctx.widget_state.request_accessibility = true;
diff --git a/masonry/src/widget/text_area.rs b/masonry/src/widget/text_area.rs
index 9a61c4584..399f0cf83 100644
--- a/masonry/src/widget/text_area.rs
+++ b/masonry/src/widget/text_area.rs
@@ -788,17 +788,8 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
 
     fn update(&mut self, ctx: &mut UpdateCtx, event: &Update) {
         match event {
-            Update::FocusChanged(focused) => {
-                // HACK: currently, when moving focus away from a text area, the Ime::Disabled
-                // event is routed to the newly focused widget. Do IME clean up here.
-                if !focused && self.editor.is_composing() {
-                    let (fctx, lctx) = ctx.text_contexts();
-                    self.editor.transact(fctx, lctx, |txn| txn.clear_compose());
-                    self.rendered_generation = self.editor.generation();
-                    ctx.request_layout();
-                } else {
-                    ctx.request_render();
-                }
+            Update::FocusChanged(_) => {
+                ctx.request_render();
             }
             Update::DisabledChanged(_) => {
                 // We might need to use the disabled brush, and stop displaying the selection.

From 6a5abfcdc1cec86fa61efd15b2536f5e525deeaa Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 29 Nov 2024 14:43:13 +0100
Subject: [PATCH 08/14] Import consistency

---
 masonry/src/passes/update.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index c341e8852..626426229 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -6,7 +6,7 @@ use std::collections::HashSet;
 use cursor_icon::CursorIcon;
 use tracing::{info_span, trace};
 
-use crate::passes::event::run_on_pointer_event_pass;
+use crate::passes::event::{run_on_pointer_event_pass, run_on_text_event_pass};
 use crate::passes::{enter_span, enter_span_if, merge_state_up, recurse_on_children};
 use crate::render_root::{RenderRoot, RenderRootSignal, RenderRootState};
 use crate::tree_arena::ArenaMut;
@@ -15,8 +15,6 @@ use crate::{
     WidgetState,
 };
 
-use super::event::run_on_text_event_pass;
-
 // --- MARK: HELPERS ---
 fn get_id_path(root: &RenderRoot, widget_id: Option<WidgetId>) -> Vec<WidgetId> {
     let Some(widget_id) = widget_id else {

From 7bdc3b1b578fe7292982d126a9e866a1c65db3af Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 29 Nov 2024 16:00:50 +0100
Subject: [PATCH 09/14] Attempt 2 at handling of event handling effects inside
 focus update

---
 masonry/src/passes/update.rs | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index 626426229..d1d4a0664 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -403,6 +403,19 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
     }
 
     let prev_focused = root.global_state.focused_widget;
+    let was_ime_active = root.global_state.is_ime_active;
+
+    let synthesize_ime_disabled =
+        was_ime_active && prev_focused != root.global_state.next_focused_widget;
+    if synthesize_ime_disabled {
+        // IME was active, but the next focused widget is going to receive the
+        // Ime::Disabled event. Synthesize an Ime::Disabled event here and send it to
+        // the widget about to be unfocused.
+        run_on_text_event_pass(root, &TextEvent::Ime(winit::event::Ime::Disabled));
+    }
+
+    // Note: handling of the Ime::Disabled event sent above may have changed the next focused
+    // widget.
     let next_focused = root.global_state.next_focused_widget;
 
     // "Focused path" means the focused widget, and all its parents.
@@ -459,8 +472,10 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         }
     }
 
-    if prev_focused != next_focused {
-        let was_ime_active = root.global_state.is_ime_active;
+    // Refocus if the focused widget changed. Or, if the focused widget was going to change, the
+    // handling of the synthesized Ime::Disabled event above may have changed the focus back to the
+    // original widget. In that case, also refocus. This is messy.
+    if prev_focused != next_focused || synthesize_ime_disabled {
         let is_ime_active = if let Some(id) = next_focused {
             root.widget_arena.get_state(id).item.accepts_text_input
         } else {
@@ -472,12 +487,6 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         // We also request accessibility, because build_access_node() depends on the focus state.
         if let Some(prev_focused) = prev_focused {
             if root.widget_arena.has(prev_focused) {
-                if was_ime_active {
-                    // IME was active, but the next focused widget is going to receive the
-                    // Ime::Disabled event. Synthesize an Ime::Disabled event here and send it to
-                    // the widget about to be unfocused.
-                    run_on_text_event_pass(root, &TextEvent::Ime(winit::event::Ime::Disabled));
-                }
                 run_single_update_pass(root, prev_focused, |widget, ctx| {
                     widget.update(ctx, &Update::FocusChanged(false));
                     ctx.widget_state.request_accessibility = true;
@@ -508,7 +517,7 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         }
     }
 
-    root.global_state.focused_widget = root.global_state.next_focused_widget;
+    root.global_state.focused_widget = next_focused;
     root.global_state.focused_path = next_focused_path;
 }
 

From a8b1c240235606a570bb5f43ea1aff10625c6dd6 Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 29 Nov 2024 21:29:57 +0100
Subject: [PATCH 10/14] Simplify IME handling in focus pass

---
 masonry/src/passes/update.rs | 42 ++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index d1d4a0664..11c197b12 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -405,13 +405,13 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
     let prev_focused = root.global_state.focused_widget;
     let was_ime_active = root.global_state.is_ime_active;
 
-    let synthesize_ime_disabled =
-        was_ime_active && prev_focused != root.global_state.next_focused_widget;
-    if synthesize_ime_disabled {
-        // IME was active, but the next focused widget is going to receive the
-        // Ime::Disabled event. Synthesize an Ime::Disabled event here and send it to
-        // the widget about to be unfocused.
+    let disable_ime = was_ime_active && prev_focused != root.global_state.next_focused_widget;
+    if disable_ime {
+        // IME was active, but the next focused widget is going to receive the Ime::Disabled event
+        // sent by the platform. Synthesize an Ime::Disabled event here and send it to the widget
+        // about to be unfocused.
         run_on_text_event_pass(root, &TextEvent::Ime(winit::event::Ime::Disabled));
+        root.global_state.emit_signal(RenderRootSignal::EndIme);
     }
 
     // Note: handling of the Ime::Disabled event sent above may have changed the next focused
@@ -475,14 +475,7 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
     // Refocus if the focused widget changed. Or, if the focused widget was going to change, the
     // handling of the synthesized Ime::Disabled event above may have changed the focus back to the
     // original widget. In that case, also refocus. This is messy.
-    if prev_focused != next_focused || synthesize_ime_disabled {
-        let is_ime_active = if let Some(id) = next_focused {
-            root.widget_arena.get_state(id).item.accepts_text_input
-        } else {
-            false
-        };
-        root.global_state.is_ime_active = is_ime_active;
-
+    if prev_focused != next_focused || disable_ime {
         // We send FocusChange event to widget that lost and the widget that gained focus.
         // We also request accessibility, because build_access_node() depends on the focus state.
         if let Some(prev_focused) = prev_focused {
@@ -501,19 +494,20 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
                 ctx.widget_state.request_accessibility = true;
                 ctx.widget_state.needs_accessibility = true;
             });
-        }
 
-        if prev_focused.is_some() && was_ime_active {
-            root.global_state.emit_signal(RenderRootSignal::EndIme);
-        }
-        if next_focused.is_some() && is_ime_active {
-            root.global_state.emit_signal(RenderRootSignal::StartIme);
-        }
+            let widget_state = root.widget_arena.get_state(next_focused).item;
+
+            root.global_state.is_ime_active = widget_state.accepts_text_input;
+            if widget_state.accepts_text_input {
+                root.global_state.emit_signal(RenderRootSignal::StartIme);
+            }
 
-        if let Some(id) = next_focused {
-            let ime_area = root.widget_arena.get_state(id).item.get_ime_area();
             root.global_state
-                .emit_signal(RenderRootSignal::new_ime_moved_signal(ime_area));
+                .emit_signal(RenderRootSignal::new_ime_moved_signal(
+                    widget_state.get_ime_area(),
+                ));
+        } else {
+            root.global_state.is_ime_active = false;
         }
     }
 

From 4737b243d2d1543eb0bc51df84478c6a8b5bed63 Mon Sep 17 00:00:00 2001
From: Tom Churchman <thomas@kepow.org>
Date: Sat, 30 Nov 2024 12:17:24 +0100
Subject: [PATCH 11/14] Keep messy logic local

Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
---
 masonry/src/passes/update.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index 11c197b12..72de3e229 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -411,7 +411,18 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         // sent by the platform. Synthesize an Ime::Disabled event here and send it to the widget
         // about to be unfocused.
         run_on_text_event_pass(root, &TextEvent::Ime(winit::event::Ime::Disabled));
+        // Disable the IME, which was enabled specifically for this widget.
+        // Note that if the newly focused widget also requires IME, we will request it 
+        // again - this resets the platform's state, ensuring that partial IME 
+        // inputs do not "travel" between widgets
         root.global_state.emit_signal(RenderRootSignal::EndIme);
+        if prev_focused == root.global_state.next_focused_widget {
+             tracing::warn!(id = prev_focused.map(|id| id.trace()), "request_focus called whilst handling Ime::Disabled");
+             // In this unlikely case, the rest of this handler will short-circuit, and IME will never be re-enabled 
+             // for this widget. The resultant `ImeEnabled` event will be routed to this widget as it is the focused
+             // widget. We don't handle this as above to avoid loops
+            root.global_state.emit_signal(RenderRootSignal::StartIme);
+        }
     }
 
     // Note: handling of the Ime::Disabled event sent above may have changed the next focused

From 28ccc0071d81cab4668e72294d2f127cca55c370 Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Sat, 30 Nov 2024 12:32:48 +0100
Subject: [PATCH 12/14] Clean up and docs

---
 masonry/src/passes/update.rs | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index 72de3e229..2da0e543f 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -405,22 +405,30 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
     let prev_focused = root.global_state.focused_widget;
     let was_ime_active = root.global_state.is_ime_active;
 
-    let disable_ime = was_ime_active && prev_focused != root.global_state.next_focused_widget;
-    if disable_ime {
+    if was_ime_active && prev_focused != root.global_state.next_focused_widget {
         // IME was active, but the next focused widget is going to receive the Ime::Disabled event
-        // sent by the platform. Synthesize an Ime::Disabled event here and send it to the widget
+        // sent by the platform. Synthesize an `Ime::Disabled` event here and send it to the widget
         // about to be unfocused.
         run_on_text_event_pass(root, &TextEvent::Ime(winit::event::Ime::Disabled));
-        // Disable the IME, which was enabled specifically for this widget.
-        // Note that if the newly focused widget also requires IME, we will request it 
-        // again - this resets the platform's state, ensuring that partial IME 
-        // inputs do not "travel" between widgets
+
+        // Disable the IME, which was enabled specifically for this widget. Note that if the newly
+        // focused widget also requires IME, we will request it again - this resets the platform's
+        // state, ensuring that partial IME inputs do not "travel" between widgets
         root.global_state.emit_signal(RenderRootSignal::EndIme);
+
+        // Note: handling of the Ime::Disabled event sent above may have changed the next focused
+        // widget. In particular, focus may have changed back to the original widget we just
+        // disabled IME for.
+        //
+        // In this unlikely case, the rest of this handler will short-circuit, and IME would not be
+        // re-enabled for this widget. Re-enable IME here; the resultant `Ime::Enabled` event sent
+        // by the platform will be routed to this widget as it remains the focused widget. We don't
+        // handle this as above to avoid loops.
         if prev_focused == root.global_state.next_focused_widget {
-             tracing::warn!(id = prev_focused.map(|id| id.trace()), "request_focus called whilst handling Ime::Disabled");
-             // In this unlikely case, the rest of this handler will short-circuit, and IME will never be re-enabled 
-             // for this widget. The resultant `ImeEnabled` event will be routed to this widget as it is the focused
-             // widget. We don't handle this as above to avoid loops
+            tracing::warn!(
+                id = prev_focused.map(|id| id.trace()),
+                "request_focus called whilst handling Ime::Disabled"
+            );
             root.global_state.emit_signal(RenderRootSignal::StartIme);
         }
     }
@@ -483,10 +491,8 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         }
     }
 
-    // Refocus if the focused widget changed. Or, if the focused widget was going to change, the
-    // handling of the synthesized Ime::Disabled event above may have changed the focus back to the
-    // original widget. In that case, also refocus. This is messy.
-    if prev_focused != next_focused || disable_ime {
+    // Refocus if the focused widget changed.
+    if prev_focused != next_focused {
         // We send FocusChange event to widget that lost and the widget that gained focus.
         // We also request accessibility, because build_access_node() depends on the focus state.
         if let Some(prev_focused) = prev_focused {

From 6ebd0d55c62b1b30660467060ab53e70d22d1a0c Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Sat, 30 Nov 2024 12:34:16 +0100
Subject: [PATCH 13/14] Move interactivity check below IME handling

---
 masonry/src/passes/update.rs | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index 2da0e543f..df19b3de3 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -394,13 +394,6 @@ pub(crate) fn run_update_focus_chain_pass(root: &mut RenderRoot) {
 // --- MARK: UPDATE FOCUS ---
 pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
     let _span = info_span!("update_focus").entered();
-    // If the focused widget is disabled, stashed or removed, we set
-    // the focused id to None
-    if let Some(id) = root.global_state.next_focused_widget {
-        if !root.is_still_interactive(id) {
-            root.global_state.next_focused_widget = None;
-        }
-    }
 
     let prev_focused = root.global_state.focused_widget;
     let was_ime_active = root.global_state.is_ime_active;
@@ -433,8 +426,13 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         }
     }
 
-    // Note: handling of the Ime::Disabled event sent above may have changed the next focused
-    // widget.
+    // If the focused widget is disabled, stashed or removed, we set
+    // the focused id to None
+    if let Some(id) = root.global_state.next_focused_widget {
+        if !root.is_still_interactive(id) {
+            root.global_state.next_focused_widget = None;
+        }
+    }
     let next_focused = root.global_state.next_focused_widget;
 
     // "Focused path" means the focused widget, and all its parents.

From c126f20156dbf36cc7fc613bad5481d0669e7d7c Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Sat, 30 Nov 2024 12:56:14 +0100
Subject: [PATCH 14/14] Check for widget interactivity again

---
 masonry/src/passes/update.rs | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
index df19b3de3..6444f4bb9 100644
--- a/masonry/src/passes/update.rs
+++ b/masonry/src/passes/update.rs
@@ -394,6 +394,13 @@ pub(crate) fn run_update_focus_chain_pass(root: &mut RenderRoot) {
 // --- MARK: UPDATE FOCUS ---
 pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
     let _span = info_span!("update_focus").entered();
+    // If the next-focused widget is disabled, stashed or removed, we set
+    // the focused id to None
+    if let Some(id) = root.global_state.next_focused_widget {
+        if !root.is_still_interactive(id) {
+            root.global_state.next_focused_widget = None;
+        }
+    }
 
     let prev_focused = root.global_state.focused_widget;
     let was_ime_active = root.global_state.is_ime_active;
@@ -417,6 +424,13 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         // re-enabled for this widget. Re-enable IME here; the resultant `Ime::Enabled` event sent
         // by the platform will be routed to this widget as it remains the focused widget. We don't
         // handle this as above to avoid loops.
+        //
+        // First do the disabled, stashed or removed check again.
+        if let Some(id) = root.global_state.next_focused_widget {
+            if !root.is_still_interactive(id) {
+                root.global_state.next_focused_widget = None;
+            }
+        }
         if prev_focused == root.global_state.next_focused_widget {
             tracing::warn!(
                 id = prev_focused.map(|id| id.trace()),
@@ -426,13 +440,6 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
         }
     }
 
-    // If the focused widget is disabled, stashed or removed, we set
-    // the focused id to None
-    if let Some(id) = root.global_state.next_focused_widget {
-        if !root.is_still_interactive(id) {
-            root.global_state.next_focused_widget = None;
-        }
-    }
     let next_focused = root.global_state.next_focused_widget;
 
     // "Focused path" means the focused widget, and all its parents.