Search for JPanel in optionPane that contains the buttons

[daniel-theia]

Related issue: #218

[daniel-theia]

@andrasfuchs Can I trouble you for a review of this please? It affects command line usage in Ubuntu (maybe others) in v1.8.0: #218

It appears at some point Java changed the layout of the option pane dialogue. Rather than assume we know where the buttons are, this performs a quick search for them. This approach should also be backwards compatible encase somebody uses outdated libraries.

Not sure how much you want to engineer safety into this, we could:

• recursively search the dialogue for buttons
• check the strings of the buttons to ensure they are not flipped
• have a default behaviour encase the buttons are not found

Regarding the exception handling message, it would be great if it also gave a file name and line number for where the exception was triggered.

[andrasfuchs]

Hey @daniel-theia, thank you for the PR, it sounds great, and I appreciate your help!

I'm on holiday at the moment, and I need to focus on other things too as soon as I'm back, but I plan to block a day or two for freerouting in the near future. When this happens your PR will be one of the first things to check.

Thanks again, I'll keep you updated here.

[maksz42]

@daniel-theia Hi, I haven't come across the issue but looks like the commit e1fd77a in my PR #224 might have fixed the problem. Instead of searching for the button I supplied the dialog with my own. Can you check if that works?

[daniel-theia]

@daniel-theia Hi, I haven't come across the issue but looks like the commit e1fd77a in my PR #224 might have fixed the problem. Instead of searching for the button I supplied the dialog with my own. Can you check if that works?

@maksz42 Haven't currently got time to check, but looking through the code it does appear to address the casting issue from the wrong index.

[andrasfuchs]

@daniel-theia I didn't find the part where you check if the buttons are flipped. Their text might mislead that check because freerouting applies the local language by default. I also added a check for the case where we don't find an optionPanel to prevent an exception.

Otherwise it looks good, thank you, I'll merge it now.

[daniel-theia]

@andrasfuchs : I didn't find the part where you check if the buttons are flipped. Their text might mislead that check because freerouting applies the local language by default.

We find the buttons: https://github.com/daniel-theia/freerouting/blob/748b390772e197534f94d453233d2cc077b779f3/src/main/java/app/freerouting/gui/MainApplication.java#L371

And then we set the text to the locale, meaning we control their text meaning. As long as the locales are all stored together (which they should be), there is no possibility for them being flipped.

Let me check this merge, I see a difference between the start and cancel button setup and want to make sure that was pushed correctly.

[daniel-theia]

Hopefully some useful comments on the changes.

[daniel-theia]

cancelButton is declared in name only outside of the if-statement, whereas startButton is fully declared. I know that startButton is not used in a timer, but for readability it makes sense to have both of these buttons similar so that in the future they are coded in the same way.

[andrasfuchs]

Good insight!

[daniel-theia]

Should make sure that there is more than one component packed into the JPanel first, something like:

if(
  c instanceof JPanel &&
  ((JPanel)c).getComponents().length >= 2 && // Ensure that there is at least two components
  ((JPanel)c).getComponents()[0] instanceof JButton &&
  ((JPanel)c).getComponents()[1] instanceof JButton
){

[andrasfuchs]

I agree 👍

[andrasfuchs]

I've made these modifications.

Thank you!

[daniel-theia]

@andrasfuchs No problem, thank you.