toga icon indicating copy to clipboard operation
toga copied to clipboard

MultilineTextInput: on_change event firing when creating widget

Open JDarsley opened this issue 9 months ago • 2 comments

Describe the bug

When declarng a MultilineTextInput, and setting an initial value, the on_change event is being fired. (That doesn't happen with a TextInput or DateInput.)

events4.py:

import toga
from toga.style import Pack
from toga.style.pack import CENTER, COLUMN, ROW, START, RIGHT, TOP, BOTTOM, SERIF, BOLD

class Jotter(toga.App):

    def startup(self):
        self.isInitialised = False

        inpSubject = toga.TextInput(
                                    #readonly=True,
                                    placeholder="Subject",     # only takes effect if readOnly=False
                                    value="Subject Text",
                                    on_change = self.edited
        )

        inpNote = toga.MultilineTextInput(
                                    #readonly=True,
                                    placeholder="Note",     # only takes effect if readOnly=False
                                    value="Note Text",
                                    on_change = self.edited
        )


        self.boxEditor = toga.Box(
            children=[
                inpSubject,
                inpNote
            ],
            style=Pack(direction=COLUMN,
                       margin=(40, 40)
                        )
        )
        self.main_window = toga.MainWindow(title=self.formal_name,
                                           #size=(self.MAIN_WIDTH, self.MAIN_HEIGHT)
                                           )

        self.main_window.content = self.boxEditor

        self.main_window.show()
        self.isInitialised = True

    def edited(self, widget, **kwargs):
        print(f"Edited: {widget.value}")

        # The final input field (is it because it's a multiline input?) triggers a call
        # to this function when its value is first set before the screen is displayed.
        if self.isInitialised:
            print("Field has been edited")
        else:
            print("Initialisation not yet completed")


if __name__ == '__main__':
    app = Jotter("Events", "org.jd.jotter")
    app.main_loop()        

Steps to reproduce

  1. Open a console.
  2. python events4.py
  3. The console will show that the event handler has run for the multiline inpNote field, but not for the TextInput field.
  4. (Any subsequent changes to either field will trigger the event.)

Expected behavior

I don't think that setting an initial value when creating a MultilineTextInput field should trigger the on_change event. It doesn't do so for TextInput or DateInput fields.

Screenshots

No response

Environment

  • Operating System: Windows 11 24H2
  • Python version: 3.12.6
  • Software versions:
    • Briefcase: 0.3.22
    • Toga: proactor-exit branch

Logs


Additional context

No response

JDarsley avatar Mar 23 '25 22:03 JDarsley

Interestingly, I can't reproduce this on a Mac. This makes me suspect it's in the Windows backend specifically.

HalfWhitt avatar Mar 24 '25 00:03 HalfWhitt

I can reproduce this on Windows; agreed that it's not desirable behavior.

I'm not entirely clear why it's happening though. The Winforms event that is causing the problem appears to be generated by adding the native widget to the parent box; I have no idea why that would be triggering an event, though.

One fix that seems to work is to add an extra condition to the winforms_text_changed handler so that it won't fire unless the native widget has a parent (i.e., self.native.Parent is not None). However, that would also be a fairly major change to event handling semantics - the widget wouldn't fire a signal unless it was part of a widget hierarchy.

freakboy3742 avatar Mar 24 '25 05:03 freakboy3742

Supposedly, Winfroms TextChanged event will be raised if the Text property (value) is changed by either a programmatic modification or user interaction. The problem seems to be related to the initialization of the RichFieldBox (Windows Backend for MultilineTextInput). After adding it to the parent box for the first time, removing it and adding it to other parent boxes would not raise the TextChanged event. It could have something to do with RichFieldBox's support for loading RTF or plain text files directly. Maybe it's doing something like assigning the Text property to its current value in the backend, which would raise a TextChangedevent even if the content looks exactly the same before and after.

albuszheng avatar May 29 '25 21:05 albuszheng

It could have something to do with RichFieldBox's support for loading RTF or plain text files directly. Maybe it's doing something like assigning the Text property to its current value in the backend, which would raise a TextChangedevent even if the content looks exactly the same before and after.

That definitely sounds plausible - I'm not sure it suggests any avenues for addressing the problem, though. The only other approach that is coming to mind right now is to track the initial value provided at time of construction and silence the on_change if the value hasn't actually changed...

freakboy3742 avatar May 30 '25 06:05 freakboy3742

I think I have figured out the reason for that unexpected event firing. It has something to do with Toga's process of adapting a scalable object when it is registered as a child of another widget. More specifically, the change of font size. For some reason (probably related to its support of RTF), RichFieldBox treats any change to the font as a change to the text, which raises the TextChanged event. This chain of events happened in the scale_font() method call for MultilineTextInput, which inherited the method from the Widget class.

I don't think knowing the cause can give us some ideas for a solution. Since the root cause is tied to what qualifies as text (value) changes to RichTextField. It, however, exposes another potential issue that any style change to the content of a MultilineTextInput widget will raise an on_change event on Windows.

The proposed temporary fix should work in this case, but it does not address the root cause. One suggestion I have is to expose the GotFocus and LostFocus events. This way, developers could track the changes to the value of the MultilineTextInput when users finish editing the content or move the focus away from the widget, without the need to constantly deal with on_change events during users' input actions.

albuszheng avatar Jun 04 '25 06:06 albuszheng

I think I have figured out the reason for that unexpected event firing. It has something to do with Toga's process of adapting a scalable object when it is registered as a child of another widget. More specifically, the change of font size. For some reason (probably related to its support of RTF), RichFieldBox treats any change to the font as a change to the text, which raises the TextChanged event. This chain of events happened in the scale_font() method call for MultilineTextInput, which inherited the method from the Widget class.

Interesting... out of interest, if you completely disable font handling (commenting out the implementation of set_font() on the base Widget - do we still get the second event?

The reason I ask - set_font() will be invoked at least once when an app is created; so what we could be seeing is a side effect of a "font change". If changing a font is triggering a content change, then programmatically setting a font on a MultilineTextInput should trigger an on_change event - which would also be a bug.

However, it's would be a bug that also gives us a way in - if MultilineTextInput overrides set_font(), and either temporarily removes the winforms-level event handler, or sets a temporary flag that prevents on_change from being invoked - we've got a way to prevent the second event from occurring.

The proposed temporary fix should work in this case, but it does not address the root cause. One suggestion I have is to expose the GotFocus and LostFocus events. This way, developers could track the changes to the value of the MultilineTextInput when users finish editing the content or move the focus away from the widget, without the need to constantly deal with on_change events during users' input actions.

Having gain and lose focus events may be worthwhile, but I don't think we should be expecting users to manually manage focus events to determine whether content has actually changed.

freakboy3742 avatar Jun 04 '25 07:06 freakboy3742

if you completely disable font handling (commenting out the implementation of set_font() on the base Widget - do we still get the second event?

I can confirm that disabling font handling as suggested will not affect the expected behavior of on_change events and remove the undesired event triggering during the setup of the widget. The bad news is that there are more style changes that could trigger an on_change event on a MultilineTextInput on Windows, and some of them do not fall under the scope of font handling. The following code snippet shows a non-exhaustive list of style changes that can be made programmatically to trigger an on_change event; It should be reproducible by adding it to the startup() method in the example shown above.

        ...
        inpNote.style.font_family = SERIF
        inpNote.style.font_size = 20
        inpNote.style.font_weight = BOLD
        inpNote.style.text_align = RIGHT
        inpNote.style.color = '#0000FF'
        ...

The issue is larger than font handling alone. Do you think there is a cheap way for us to check if the text value has changed? I'm thinking maybe use hash values for this check. So we could have a flag indicating whether the change is a style change or a text change, and prevent the on_change being invoked if it's only a style change.

albuszheng avatar Jun 05 '25 00:06 albuszheng

if you completely disable font handling (commenting out the implementation of set_font() on the base Widget - do we still get the second event?

I can confirm that disabling font handling as suggested will not affect the expected behavior of on_change events and remove the undesired event triggering during the setup of the widget.

🎉

The bad news is that there are more style changes that could trigger an on_change event on a MultilineTextInput on Windows, and some of them do not fall under the scope of font handling. The following code snippet shows a non-exhaustive list of style changes that can be made programmatically to trigger an on_change event; It should be reproducible by adding it to the startup() method in the example shown above.

        ...
        inpNote.style.font_family = SERIF
        inpNote.style.font_size = 20
        inpNote.style.font_weight = BOLD
        inpNote.style.text_align = RIGHT
        inpNote.style.color = '#0000FF'
        ...

The issue is larger than font handling alone. Do you think there is a cheap way for us to check if the text value has changed? I'm thinking maybe use hash values for this check. So we could have a flag indicating whether the change is a style change or a text change, and prevent the on_change being invoked if it's only a style change.

So - the good news is that these are all style changes - which means they all have clear entry points. All the font changes go through set_font(); color is set_color(), and alignment is set_text_align(). So - whatever mechanism we use to flag a "style change" should world for all these methods.

I can see two ways to target this:

The first is content based, as you've suggested; if we track the actual content, and only fire on_change if the content has changed, that should prevent the problem. Hashes might be one way to do that, but honestly, just tracking the full value of self.native.Text at the time of the last event is probably sufficient. If a hash can be computed quickly, then that might be worth it to save memory.

The other is to use a state variable around style. We could use a state variable that is set whenever style changes, and use that to negate sending the next change event... but then there's interactions with content (both programmatic and user-initiated) to consider.

freakboy3742 avatar Jun 05 '25 04:06 freakboy3742

I made a fix using the content based solution, directly comparing the full value of self.native.Text. I found that there are more TextChanged events fired by Winfroms.RichFieldBox, which makes using state variables more complicated. Some of these events are already suppressed during the initialization process.

Additionally, could you expand on your concern about the interactions with content regarding programmatic and user-initiated? To the best of my knowledge, the style setting should behave the same regardless of whether the change is programmatic or user-initiated.

albuszheng avatar Jun 07 '25 05:06 albuszheng

Additionally, could you expand on your concern about the interactions with content regarding programmatic and user-initiated? To the best of my knowledge, the style setting should behave the same regardless of whether the change is programmatic or user-initiated.

It wasn't a concern about the style settings - it was more of a concern about the fact that there's (apparently) three possible sources of change that can trigger a winforms change event:

  1. A programmatica set_value() in Toga
  2. A change in style (programmatic, in Toga)
  3. A user typing a key when the widget has focus.

It's easy enough to maintain a clear concept of state for the first two; but when the user types into the widget, there's no corresponding Toga API entry point. If you set a state variable to ignore updates because of a programmatic style change, and then the user types a key, you do need to propagate the event. If the two events occur far enough apart chronologically, then that's no real problem - but it's also easy enough to imagine that they could (and hard enough to prove that they couldn't) occur at the same time and only generate a single winforms event.

freakboy3742 avatar Jun 07 '25 06:06 freakboy3742