search

trodi / electron-splashscreen

Simple splashscreen for electron applications.
MIT License
170 stars 29 forks source link

Javascript Error on early process termination #25

Closed neil-benn closed 3 years ago

neil-benn commented 3 years ago

Description

Browser window is null on close

Environment

Steps to Reproduce

start the application from the command line and control C before the main widow appears.

Expected behaviour

The application should close

Actual behaviour

You get a modal dialog displaying a javascript error. I believe that this is because of the close event calling mainwindow.close before the main window is set. It would be easy to put a null check on that call.

trodi commented 3 years ago

@neil-benn I don't have a Ubuntu 20 environment to test. However, I tested on electron 6.1.7 and 8.1.1 on my Mac attempting to reproduce, which I was unable. What is the exact JS error? Also I'm happy to accept a PR to fix your issue.

neil-benn commented 3 years ago

Hello,

In fact I fixed it already - the section for close was changed to:

xConfig.closeWindow && splashScreen.on("close", () => { try { done || window.isDestroyed() || window.close(); } catch (e) { // don't care } });

I didn't update because the last push was a while back and I wan't sure if you were still keeping it going. This got rid of the eror; note on teh other ticket I posted this is not an issue for transparraency on election splash screen but the underlying electron library. This can actually be solved by putting a small 500ms wait before the splashscreen is shown - not really an electron-splashscreen problem.

Cheers,

Neil

On Sun, 18 Jul 2021 at 21:55, Troy McKinnon @.***> wrote:

@neil-benn https://github.com/neil-benn I don't have a Ubuntu 20 environment to test. However, I tested on electron 6.1.7 and 8.1.1 on my Mac attempting to reproduce, which I was unable. What is the exact JS error? Also I'm happy to accept a PR to fix your issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/trodi/electron-splashscreen/issues/25#issuecomment-882115453, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADY2NXT6DCQWURZVVYN4OXLTYM5VLANCNFSM476QGJQA .

--

Neil Benn MSc Ziath Ltd Phone: +44 (0) 1223 855021 http://www.ziath.com

Please consider the environment before printing this email.

Follow us on Facebook http://www.facebook.com/ziathltd, Twitter http://www.twitter.com/ziath_ltd or LinkedIn http://www.linkedin.com/company/ziath-ltd

IMPORTANT NOTICE: This message, including any attached documents, is intended only for the use of the individual or entity to which it is addressed, and may contain information that is privileged, confidential and exempt from disclosure under applicable law. If the reader of this message is not the intended recipient, or the employee or agent responsible for delivering the message to the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify Ziath Ltd immediately by email at @.*** Thank you.

trodi commented 3 years ago

This looks like #19 but on the main window instead of the splashscreen. Can you provide a screenshot of the JS error so I can confirm? Then I'll release a patch.

I didn't update because the last push was a while back and I wan't sure if you were still keeping it going.

This project is still maintained. There hasn't been a need to make updates as it has been in production and feature complete without issue for awhile.

note on teh other ticket I posted ... this is not an issue for transparraency on election splash screen but the underlying electron library. This can actually be solved by putting a small 500ms wait before the splashscreen is shown - not ... an electron-splashscreen problem. (#24)

Electron has had some limitations with transparent windows (#12 #10). Glad you found the linux resolution.

neil-benn commented 3 years ago

Hello,

It is an exact copy of #19, I copied the code from there to fix this one. It'll take me a few days to get the error though.

Cheers,

Neil

On Tue, 20 Jul 2021, 04:22 Troy McKinnon, @.***> wrote:

This looks like #19 https://github.com/trodi/electron-splashscreen/issues/19 but on the main window instead of the splashscreen. Can you provide a screenshot of the JS error so I can confirm? Then I'll release a patch.

I didn't update because the last push was a while back and I wan't sure if you were still keeping it going.

This project is still maintained. There hasn't been a need to make updates as it has been in production and feature complete without issue for awhile.

note on teh other ticket I posted ... this is not an issue for transparraency on election splash screen but the underlying electron library. This can actually be solved by putting a small 500ms wait before the splashscreen is shown - not ... an electron-splashscreen problem. (#24 https://github.com/trodi/electron-splashscreen/issues/24)

Electron has had some limitations with transparent windows (#12 https://github.com/trodi/electron-splashscreen/issues/12 #10 https://github.com/trodi/electron-splashscreen/issues/10). Glad you found the linux resolution.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/trodi/electron-splashscreen/issues/25#issuecomment-883021339, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADY2NXR76A5LTN6OTQ7FM63TYTTW5ANCNFSM476QGJQA .

trodi commented 3 years ago

@neil-benn please verify that PR #26 fixes this issue, then I'll release v1.0.1 with the fix.

neil-benn commented 3 years ago

Hello,

Yes; thats good thanks. Will you also deploy the artifact into the main npm registry?

Cheers,

Neil

On Wed, 21 Jul 2021 at 03:33, Troy McKinnon @.***> wrote:

@neil-benn https://github.com/neil-benn please verify that PR #26 https://github.com/trodi/electron-splashscreen/pull/26 fixes this issue, then I'll release v1.0.1 with the fix.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/trodi/electron-splashscreen/issues/25#issuecomment-883838833, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADY2NXRHC36ZQF5IESK4QDLTYYWXVANCNFSM476QGJQA .

--

Neil Benn MSc Ziath Ltd Phone: +44 (0) 1223 855021 http://www.ziath.com

Please consider the environment before printing this email.

Follow us on Facebook http://www.facebook.com/ziathltd, Twitter http://www.twitter.com/ziath_ltd or LinkedIn http://www.linkedin.com/company/ziath-ltd

IMPORTANT NOTICE: This message, including any attached documents, is intended only for the use of the individual or entity to which it is addressed, and may contain information that is privileged, confidential and exempt from disclosure under applicable law. If the reader of this message is not the intended recipient, or the employee or agent responsible for delivering the message to the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify Ziath Ltd immediately by email at @.*** Thank you.

trodi commented 3 years ago

Yes, it is published as v1.0.2