-
Notifications
You must be signed in to change notification settings - Fork 182
[GTK] Fix Combo copy/paste tests on Gtk 4.x #2444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test Results 485 files - 61 485 suites - 61 19m 42s ⏱️ - 13m 28s For more details on these failures, see this check. Results for commit 035a973. ± Comparison against base commit cde95ee. This pull request removes 91 tests.
♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the change only affects GTK4, it is fine to go in RC2. You have my approval for that.
Regarding the change itself: if there is no way around this, it's probably fine. But still it seems a bit dangerous to me as this will spin the event loop at some rather unexpected place. As far as I understand, the paste()
method on a combo can be called from anywhere in the UI thread, such that this will start spinning the event loop at an arbitrary place. We attempted to do something similar in Edge browser and concluded that it's not a good idea (and also implemented some alternative): #1773
On Windows it's possible to peek specific messages from the OS to not process everything but only the kind of event you are waiting for. But I don't think that's possible on GTK, so probably there is no way around spinning the whole event loop.
I agree with @HeikoKlare that spinning the event queue is not really nice and wondering why it is required at all should this not happen anyways (sooner or later) and is the problem probably more that one expect the past happen instantly? Should we probably just enhance the javadoc that the paste can happen asynchronously and any code that relies on the happen-before should use a display.asyncExec call before access the widget state after that? |
Gtk 4 no longer provides API for doing that but rather a built-in action to activate. This requires to spin the event loop to process it.
558f965
to
035a973
Compare
I assume this can be closed now, superceeded by #2489 |
This is considered too risky so a documentation is being added and tests event loop spinned instead. |
Gtk 4 no longer provides API for doing that but rather a built-in action to activate. This requires to spin the event loop to process it.