Fix randomly blank currency box upon opening the Offer screen

[cparke2]

When clicking Create Buy/Sell Offer, or Edit/Duplicate/Clone an existing offer, randomly the initial display of "Currency" associated with the initially selected payment method was displayed as blank. This seems to have been caused by scheduling the selection of the initial payment method combobox selection to happen on a different thread rather than immediately selecting it (apparently, sometimes the secondary event to select the currency gets lost).

[cparke2]

Bug condition being fixed: (notice how CURRENCY is blank)

[HenrikJannsen]

Did this change avoid that issue? The UserThread.execute schedules the execution to the next render frame and serves as a small delay which often fixed UI synchronisation issues.

[cparke2]

Yes, it does seem to have fixed the issue. From all the test scenarios that I could think of, it did not seem to break anything! I'm aware what UserThread.execute() does, as well as that it was used uncharacteristically here (which was suspicious and warranted further caution). I also do not understand why this change makes a difference like it does (for me). Most places are just fine doing a combobox select() directly, so would be interested if the dev who originally added that is still available to contact, and if he/she recalls why they chose to put userThread.execute() there?

[HenrikJannsen]

Yes, it does seem to have fixed the issue. From all the test scenarios that I could think of, it did not seem to break anything! I'm aware what UserThread.execute() does, as well as that it was used uncharacteristically here (which was suspicious and warranted further caution). I also do not understand why this change makes a difference like it does (for me). Most places are just fine doing a combobox select() directly, so would be interested if the dev who originally added that is still available to contact, and if he/she recalls why they chose to put userThread.execute() there?

The JavaFX framework works basically to queue up tasks and then execute them at a frame-rate based interval (all single thread). With UserThread.execute() we delegate some runnable code to that queue (using Platform::runlater under the hood if its a UI app). The main usage is when code is running on a non UI thread and we apply it to UI components which would then cause an exception as UI components must be called from the UI thread.
The other use case which is more a kind of cheap hack is to add a small delay for one render frame to avoid synchronization issues from UI code executions. The internal of JavaFX UI framework can become very complex and hard to debug due the event driven model and those delegations. Some flows of events can lead to complex situations with potential infinite loops or failures in updates at the render or layout phase.
To figure out exactly why some things do not behave as expected can be very difficult and time consuming. The fast hack then is to try if the delay with UserThread.execute fix the current problem. Sure often the problem is caused at the end from some incorrect usage of the framework but also to figure out where the problem is located can be difficult. So it is not uncommon that a fix by that hack might not be needed anymore later when the underlying problem was fixed (sometimes in other classes) or that it even inverts so that using it cause a new bug (might be the case here). Generally using it should not do harm as its just a tiny delay for executing things, but of course depends on the context...

[cparke2]

JavaFX isn't the only library that has such oddities; I've definitely seen this at times in pretty much every UI library, including MUI and Windows SDK itself. This could even be a bug in JavaFX itself. I'm also aware of multithreading issues and how the main event loop works, etc. from other systems, and I assumed as much was the same for JavaFX. My personal assessment of the unusual UserThread.execute() causing this issue was indeed that it was a "cheap hack" (as you put it) that someone had resorted to at one point.

Removal of UserThread does seem to resolve the issue in all my testing, but it's up to you if you care to fix this minor blemish with this pull request. Otherwise, the bug can remain, it's just a minor inconvenience that users can work around fairly easily.

[HenrikJannsen]

utACK