Skip to content
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

Test launcher mac menu #9266

Merged
merged 1 commit into from
Oct 22, 2022
Merged

Test launcher mac menu #9266

merged 1 commit into from
Oct 22, 2022

Conversation

Siedlerchr
Copy link
Member

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@Siedlerchr
Copy link
Member Author

grafik

menu works but still showing the wrong program info

@koppor
Copy link
Member

koppor commented Oct 22, 2022

MWE code for a JavaFX application https://github.com/dlemmermann/JPackageScriptFX/blob/master/jpackagefx-main/src/main/java/com/dlsc/jpackagefx/App.java - seems to be similar to the current one. In yesterday's call we guessed, it could be a timing issue.

@Siedlerchr
Copy link
Member Author

Fuu. Now it doesn't event start when I download the build version

@Siedlerchr Siedlerchr reopened this Oct 22, 2022
@tobiasdiez
Copy link
Member

Can you please set a breakpoint in https://github.com/openjdk/jfx/blob/b3eec1d655ca8d17d13615ea430e92058d99ec1c/modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java#L675, and report the differences between the current vs old startup. Thanks!

@Siedlerchr
Copy link
Member Author

Okay, now it seems to work fine for me. But normally when I run the dmg I get the security dialog. I now had to open it with the commandline first to trigger the security dialog. I will need to test if this works with signing

grafik

@Siedlerchr Siedlerchr merged commit c20c390 into main Oct 22, 2022
@Siedlerchr Siedlerchr deleted the testmain branch October 22, 2022 17:19
@Siedlerchr
Copy link
Member Author

This seems to work now. I will check again with the master then

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

@@ -45,8 +45,10 @@
*/
public class Launcher {
private static Logger LOGGER;
private static String[] ARGUMENTS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this field really necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure the original args are no modified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants