macOS: Space in Apple Korean with no syllable in progress types one space (winit#4478)

With Apple Korean 2-Set on macOS 14, Space with no syllable in progress
(a second space, or a space after a digit or punctuation) makes AppKit
call setMarkedText(" "), insertText(" "), then setMarkedText(""). winit
reports the insertText as Ime::Commit(" "), but the trailing empty
setMarkedText returns its state to the ground state, so keyDown also sends
the key as KeyboardInput with text " ". egui inserts both, and PdfCraft's
text fields got two spaces (the double space of alacritty/alacritty#8079).

vendor/winit now records that the key being handled committed IME text and
then doesn't also send it as KeyboardInput, unless AppKit forwards it as a
command through doCommandBySelector (Enter after a syllable still commits
the syllable and sends Enter). keyDown consumes the record on every key,
so keys typed after IME is disallowed are unaffected.

rust-windowing/winit#4478 aims at the same double space, but its changes
(clearing the marked text on commit, forwarding a later insertText, and
dropping doCommandBySelector's guard) don't reach this call order, and the
key loss it also fixes is already fixed by the previous change
(pending_commit). This is the narrow form for the call order observed
here; it should go upstream with #4478.

Checked with the real input methods on macOS 14.7.1 (scratch winit app, on top
of the previous change): the duplicate KeyboardInput Space is gone and
nothing else changes (Korean syllables, 한+Enter, 한+5, Japanese, Cangjie,
ABC, held keys, keys after IME is disallowed). A test fails if a
re-vendored winit loses the patch.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

apps/pdfcraft/tests/winit_macos_korean_space.rs

@@ -0,0 +1,17 @@
+//! vendor/winit patch (9) (vendor/README.md).
+
+#[test]
+fn winit_sends_a_key_committed_through_macos_ime_only_once() {
+    // Apple Korean answers Space with no syllable in progress with `setMarkedText(" ")`,
+    // `insertText(" ")` and `setMarkedText("")`. The last call resets winit 0.30.13's IME state, so
+    // the key went to the app both as `Ime::Commit(" ")` and as `KeyboardInput` with text " ", and
+    // PdfCraft's text fields got two spaces. vendor/winit doesn't send `KeyboardInput` for a key
+    // whose text was committed, unless AppKit forwards it as a command (Enter) (the goal of
+    // rust-windowing/winit#4478). A re-vendored copy without the patch would silently bring the bug
+    // back: re-apply the patch, or drop the copy once a winit 0.30 release has the fix.
+    let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../..");
+    let view = std::fs::read_to_string(root.join("vendor/winit/src/platform_impl/macos/view.rs")).unwrap();
+    assert!(view.contains("self.ivars().committed_in_key_down.set(true);"), "a commit in keyDown isn't recorded");
+    let consumed = "let had_ime_input = self.ivars().committed_in_key_down.replace(false) || had_ime_input;";
+    assert!(view.contains(consumed), "a key committed through IME is sent again as KeyboardInput");
+}

vendor/README.md

@@ -13,7 +13,7 @@ the fix. Vendoring copyleft code is never allowed (plan/adr/0001).
 | hayro-jbig2 | 0.3.0 | Apache-2.0 OR MIT | (1) at most 2^28 pixels per bitmap, page bitmap, halftone grid and grayscale image (each side could be 65535: 4.3 billion pixels decoded one by one); (2) at most 2^26 symbol pixels per symbol dictionary (a fuzzed dictionary decoded for minutes); (3) a symbol dictionary's height classes are capped at one per new symbol and its export runs at `2 × symbols + 1`, each plus `EXTRA_EMPTY_STEPS` (1024): an empty height class or a zero-length export run doesn't advance, and past the end of its data the arithmetic decoder can decode them for ever (one changed byte hung the renderer) | (1) `raster::tests::jbig2_regions_of_billions_of_pixels_are_refused`; (2) the nightly `cargo xtask fuzz` finding, which now finishes in about a second; (3) `raster::tests::jbig2_symbol_dictionaries_that_never_advance_terminate` |
 | lopdf | 0.45.0 | MIT | (1) PNG predictor rows longer than the data are refused before their buffers are allocated (a fuzzed `/Columns 4294967295` allocated two 4 GiB rows), and empty frames return without row buffers; (2) predictor row widths use saturating arithmetic or reject overflow; (3) sub-byte TIFF Predictor 2 scratch is bounded by the samples present in the input instead of the declared `/Colors`, aligned with [lopdf#589](https://github.com/J-F-Liu/lopdf/pull/589) | (1) `inspect::tests::png_predictor_rows_longer_than_the_data_are_refused_before_allocating`, `inspect::tests::empty_png_predictor_frames_do_not_allocate_rows`; (2) `inspect::tests::tiff_subbyte_predictor_row_width_overflow_is_refused`; (3) `inspect::tests::tiff_subbyte_predictor_scratch_is_bounded_by_available_samples`, `inspect::tests::tiff_subbyte_predictors_keep_components_rows_and_padding` |
 | hayro-syntax | 0.7.2 | Apache-2.0 OR MIT | (1) Page-tree cycle guard in `resolve_pages` (visited set + depth cap): a `/Kids` loop among compressed objects overflowed the stack and aborted the process; (2) `ccitt_size_ok`: CCITT images wider than 2^20 columns or over 2^28 pixels are not decoded (a fuzzed `/Columns 4294967295` allocated 4 GiB); (3) Flate/LZW predictor row widths are saturating `usize`, a zero width is refused and a row longer than the data is not allocated (a fuzzed `/Columns 9223372036854775807` wrapped to a 2^61-byte allocation and aborted); (4) the inline-image end-of-data check (`EI`) doesn't nest inside its own look-ahead and examines at most 256 candidates and steps (50 images with "EI" in their data ran for over ten minutes); (5) a stream whose `/Length` is wrong is recovered only when `stream` is the next token — scanning ahead to a later object's `stream` made an indirect appearance-state dictionary (Quartz checkboxes: `/AP /N` → `<< /Yes … /Off … >>`) render as a non-form, so the widget was skipped; (6) JPX rescaling uses an exact integer sample range for 1–32-bit components (32 shifted past u32, and floating-point maxima at 25–31 bits wrapped to zero), and refuses zero-sized rows; (7) `decode_limit`: Flate (through flate2 and the fallback inflater), LZW and RunLength never write past `MAX_DECODED_STREAM` (256 MiB), or, for the last filter of an image of known size, past twice its pixel bytes (a predictor tag byte per byte), at most 1 GiB; `Stream::decoded_within` for budgeted callers; a page's `/Contents` array shares one `MAX_DECODED_STREAM` (a 2 MB page whose content stream or 8 × 8 image inflated to 2 GB took the renderer past 3 GB in under a second; naming the stream twenty times in `/Contents`, 5 GB); (8) high-precision numbers (jsPDF MediaBox coordinates with over 19 digits) parse without overflowing the u64 mantissa: fractional digits past it are dropped, integer digits past it scale the value (a real, kept finite); (9) large dictionary indexes are built on first use and shared across repeated reads within each PDF (#307); the validated-byte cache is bounded, reader contexts remain separate, repairs and inline/content dictionaries bypass sharing, and small dictionaries stay eager; (10) decoded object-stream slots publish once under contention, cold loading rejects recursive dependencies, and xref repair serializes without holding recursive read guards; this permits one parser per native render pool with private worker caches; (11) `sync.rs`: a poisoned lock is recovered instead of unwrapped (a worker panic no longer cascades into every reader of the same PDF), and xref repair takes a blocking write lock instead of asserting a non-blocking one; (12) `SegmentList` (`util.rs`) returns `None` for an out-of-range index instead of panicking | (1) `cos/tests/writer.rs::page_tree_loops_in_object_streams_do_not_crash_readers`; (2) `raster::tests::absurd_mask_and_fax_sizes_are_refused_before_decoding`; (3) `raster::tests::absurd_predictor_columns_and_line_widths_render`; (4) `raster::tests::inline_images_with_ei_in_their_data_parse_in_linear_time`; (5) `raster::tests::indirect_appearance_state_dict_is_not_another_stream`; (6) `raster::tests::jpx_stream_bit_depth_boundaries_do_not_panic`, `raster::tests::jpx_32_bit_image_does_not_prevent_other_page_content_from_rendering`, `raster::tests::jpx_gray_rgb_and_alpha_keep_eight_bit_samples`, `raster::tests::jpx_streams_reject_out_of_range_bit_depths`; (7) `raster::tests::decompression_bombs_stop_at_the_stream_limit`, `raster::tests::page_contents_and_nested_forms_share_one_budget`; (8) `inspect::tests::jspdf_high_precision_mediabox_does_not_overflow_dimensions`, `object::number::tests::high_precision_float_does_not_overflow_mantissa` (hayro-syntax); (9) `object::dict::lazy_index_tests`, `raster::tests::large_resource_indexes_preserve_pixels_and_text`; (10) `data::tests`, `xref::tests::concurrent_cold_object_stream_reads_all_succeed`, `raster::tests::shared_parser_is_lazy_and_initializes_once_under_contention`; (11) `data::tests::panicking_initializer_wakes_waiters_and_prevents_retries`, `xref::tests::concurrent_cold_object_stream_reads_all_succeed`; (12) `util::tests::segment_list_rejects_out_of_range_indices` |
-| winit | 0.30.13 | Apache-2.0 | (1) on Windows 11 (build 22000 and later), `WM_DPICHANGED` applies the rect Windows suggests. It used to nudge the window onto the monitor `MonitorFromWindow` names, which during a drag is still the monitor being left; the nudge sent another `WM_DPICHANGED`, so a window dragged to a monitor with another scale factor grew and stayed on the wrong monitor at the wrong scale (#324). Backport of winit master 488c036a ([winit#4341](https://github.com/rust-windowing/winit/pull/4341)), which no 0.30 release has; Windows 10 keeps the old adjustment, as on winit master. (6) on macOS, `insertText` commits through IME when a composition was started (`pending_commit`, set by `setMarkedText`) rather than when marked text is left: input methods that end a composition with `unmarkText` before `insertText` lost the text, and Apple Korean dropped the key typed after a syllable (`한` then `5` or `;`); other `insertText` calls stay `KeyboardInput`; a commit outside a key press (a candidate clicked with the mouse, dictation) clears the marked text and leaves no key to swallow, as in [winit#4748](https://github.com/rust-windowing/winit/pull/4748) (without it the next key, such as Backspace, was lost). `validAttributesForMarkedText` lists the underline and marked-clause attributes, without which some input methods don't compose. Port of [winit#4650](https://github.com/rust-windowing/winit/pull/4650), the 0.30 backport of [winit#4651](https://github.com/rust-windowing/winit/pull/4651) (merged on winit master), with the underline attribute spelled as AppKit's `NSUnderlineStyleAttributeName` (`NSUnderline`). The published crate's asset files (`docs/res/*.svg`, `examples/data/*.png`) are left out, so its `window` example doesn't build | (1) `pdfcraft` `tests::winit_carries_the_windows_11_monitor_scale_fix` keeps the patch in the build; the drag itself needs two monitors with different scale factors on Windows 11; (6) `pdfcraft` `tests/winit_macos_ime_commit.rs`; composing itself needs macOS and an input method |
+| winit | 0.30.13 | Apache-2.0 | (1) on Windows 11 (build 22000 and later), `WM_DPICHANGED` applies the rect Windows suggests. It used to nudge the window onto the monitor `MonitorFromWindow` names, which during a drag is still the monitor being left; the nudge sent another `WM_DPICHANGED`, so a window dragged to a monitor with another scale factor grew and stayed on the wrong monitor at the wrong scale (#324). Backport of winit master 488c036a ([winit#4341](https://github.com/rust-windowing/winit/pull/4341)), which no 0.30 release has; Windows 10 keeps the old adjustment, as on winit master. (6) on macOS, `insertText` commits through IME when a composition was started (`pending_commit`, set by `setMarkedText`) rather than when marked text is left: input methods that end a composition with `unmarkText` before `insertText` lost the text, and Apple Korean dropped the key typed after a syllable (`한` then `5` or `;`); other `insertText` calls stay `KeyboardInput`; a commit outside a key press (a candidate clicked with the mouse, dictation) clears the marked text and leaves no key to swallow, as in [winit#4748](https://github.com/rust-windowing/winit/pull/4748) (without it the next key, such as Backspace, was lost). `validAttributesForMarkedText` lists the underline and marked-clause attributes, without which some input methods don't compose. Port of [winit#4650](https://github.com/rust-windowing/winit/pull/4650), the 0.30 backport of [winit#4651](https://github.com/rust-windowing/winit/pull/4651) (merged on winit master), with the underline attribute spelled as AppKit's `NSUnderlineStyleAttributeName` (`NSUnderline`). (9) on macOS, a key whose text was committed through IME isn't also sent as `KeyboardInput` unless AppKit forwards it as a command (Enter): Apple Korean answers Space with no syllable in progress with `setMarkedText(" ")`, `insertText(" ")`, `setMarkedText("")`, and the last call reset winit's state, so the space arrived twice. Same goal as [winit#4478](https://github.com/rust-windowing/winit/pull/4478), whose changes don't reach this call order (macOS 14); to be proposed upstream there. The published crate's asset files (`docs/res/*.svg`, `examples/data/*.png`) are left out, so its `window` example doesn't build | (1) `pdfcraft` `tests::winit_carries_the_windows_11_monitor_scale_fix` keeps the patch in the build; the drag itself needs two monitors with different scale factors on Windows 11; (6) `pdfcraft` `tests/winit_macos_ime_commit.rs`; composing itself needs macOS and an input method; (9) `pdfcraft` `tests/winit_macos_korean_space.rs`; composing itself needs macOS and Apple Korean |
 | egui-wgpu | 0.36.2 | MIT OR Apache-2.0 | (1) `winit::surface_fit`: a window's surface is configured within the device's `max_texture_dimension_2d`, both sides shrunk by one factor that egui then draws at (`pixels_per_point × render_scale`), so the whole window is drawn into the surface (on DX12 it is stretched over the window and lines up with the pointer; wgpu's GL backend copies it unscaled into a corner); the longer side becomes exactly the limit; the depth and MSAA textures follow, and a recreated surface starts again from the window's own size. egui-wgpu configured the window's size as is, and `Surface::configure` panics beyond the limit: a window restored at another monitor's scale (opening an Outlook attachment on a mixed-DPI laptop asked an 8192-pixel device for 8600 × 3423, #577) or stretched across monitors closed PdfCraft, before its window appeared when it happened at start-up ([emilk/egui#8361](https://github.com/emilk/egui/issues/8361)). The published crate leaves out egui's licence files; `LICENSE-APACHE` and `LICENSE-MIT` are from the 0.36.2 tag; (2) `winit::catch_errors`: a new window's surface is configured inside wgpu error scopes (validation, out of memory, internal), and a failure is returned as `WgpuError::ConfigureSurface` (for the root window at start-up eframe returns it as `Error::Wgpu`, which PdfCraft retries with OpenGL); nothing is kept for the window. wgpu's default handler panicked instead, and eframe configures the root surface before the app is created, so a driver that can't configure one closed PdfCraft at launch (a 2015 Intel GPU through wgpu's GL backend timed out waiting for the GPU to go idle, `GpuWaitTimeout`, #519). Unchanged, so still a panic: an error in a later reconfigure (resize, a lost surface), a device lost during the first configure (wgpu reports it only to a callback), and a window shown at zero size (first configured on resize) | (1) `pdfcraft` `tests::a_window_larger_than_the_gpu_limit_gets_a_surface_within_it`, `tests::surface_fit_never_empties_or_overflows_a_surface`, and `tests::egui_wgpu_carries_the_surface_size_fit`, which keeps the patch in the build and in the paths that size a surface; (2) `pdfcraft` `tests::a_gpu_error_while_a_window_is_set_up_is_returned_not_a_panic` (a real device: an error inside `catch_errors` is returned, not a panic) and `tests::egui_wgpu_returns_a_surface_it_cannot_configure_as_an_error`, which keeps it around `add_surface`'s configure |
 
 Temporary: hayro is the bootstrap renderer and lopdf the bootstrap inspector (ADR-0004); M2.6 replaces them with our own devices and `cos`/`model`.

vendor/winit/src/platform_impl/macos/view.rs

@@ -137,6 +137,9 @@ pub struct ViewState {
     /// True while `keyDown:` runs `interpretKeyEvents`.
     in_key_down: Cell<bool>,
 
+    /// True if the key being handled by `keyDown:` committed IME text.
+    committed_in_key_down: Cell<bool>,
+
     marked_text: RefCell<Retained<NSMutableAttributedString>>,
     accepts_first_mouse: bool,
 
@@ -445,6 +448,7 @@ declare_class!(
                 if self.ivars().in_key_down.get() {
                     // `keyDown:` clears the marked text and swallows the key that committed it.
                     self.ivars().ime_state.set(ImeState::Committed);
+                    self.ivars().committed_in_key_down.set(true);
                 } else {
                     // Committed outside a key press. No key to swallow.
                     *self.ivars().marked_text.borrow_mut() = NSMutableAttributedString::new();
@@ -526,6 +530,12 @@ declare_class!(
                 // `key_down` could result in preedit clear, so compare old and current state.
                 _ => old_ime_state != self.ivars().ime_state.get(),
             };
+            // PdfCraft patch: a key whose text was committed through IME isn't sent again as
+            // `KeyboardInput` (unless `doCommandBySelector` forwards it, as for Enter). Apple Korean
+            // answers Space with no syllable in progress with `setMarkedText(" ")`,
+            // `insertText(" ")`, then `setMarkedText("")`, which resets the state above to the
+            // ground state, so the space arrived twice (rust-windowing/winit#4478).
+            let had_ime_input = self.ivars().committed_in_key_down.replace(false) || had_ime_input;
 
             if !had_ime_input || self.ivars().forward_key_to_app.get() {
                 let key_event = create_key_event(&event, true, unsafe { event.isARepeat() });
@@ -845,6 +855,7 @@ impl WinitView {
             ime_allowed: Default::default(),
             forward_key_to_app: Default::default(),
             in_key_down: Default::default(),
+            committed_in_key_down: Default::default(),
             marked_text: Default::default(),
             accepts_first_mouse,
             _ns_window: WeakId::new(&window.retain()),