floem icon indicating copy to clipboard operation
floem copied to clipboard

Fix focus handling in multi-window applications

Open timboudreau opened this issue 8 months ago • 5 comments

This took some time to debug - starting with an application that opens a popup window, wherein the user should be able to tab between items and press a key to select and close the popup. Focus handling did not work at all in the popup window.

It turned out to be three separate issues:

  1. window_handle.process_central_messages() would discard any messages for Views with a different root than its own, so they would never be processed, so focus requests would be discarded if the main window queried the queue first
  2. ViewId attempted to cache the root view id, but would just cache the deepest parent found - so if you do something that generates an update message during construction of a tree of views, the cached value would not actually be the root view for the (not yet existing) window, and it would always return None from window_id() which relies on the (in this case bogus) root to map it to a window - so a call to request_focus() on a view too early would leave it with garbage as its root id, and no window handle would ever recognize and process it with the wrong root
  3. Not quite as strictly a bug, but I can't imagine why it would be by design: FocusGained and FocusLost events caused by a call to ViewId.request_focus()would be delivered to listeners, but a View would never receive them via event_before_children() or event_after_children() - so this one had a workaround, but the workaround - a view attaching listeners to itself - would be much less readable than a straightforward implementation of one of the event handler methods

The fix for 3. I am the least sure about:

  • It is a bit ugly (swapping an EventCx for an UpdateCx to call unconditional_view_event() and then recreating the UpdateCx)
  • It is not clear if there are other kinds of events which have the same problem of non-delivery to the view itself
  • ViewId.apply_event() might be a cleaner place to implement it (though it would mean a bunch of calling back and forth between WindowHandle and ViewId which might be less than ideal)
  • I do not stop propagation of events if event_*_children() returns EventPropagation::Stop - as long is this is about focus events, I think it is probably harmful to be able to block propagation of focus events at all - window focus events more so, since it's unlikely that one view in a tree knows for sure that none of its siblings need to know about the window losing focus - but the same logic applies more weakly to plain view focus events - it's a recipe for hard-to-diagnose focus bugs.

Since I'm a proud member of the 1990's println school of debugging, UpdateMessage now implements Debug - needed it to prove that retained messages really were eventually processed, and it will probably be useful in the future.

timboudreau avatar Jun 05 '24 19:06 timboudreau