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

Multiple close confirmation handlers #1894

Open
2 tasks done
FarSeeing opened this issue Oct 22, 2024 · 0 comments
Open
2 tasks done

Multiple close confirmation handlers #1894

FarSeeing opened this issue Oct 22, 2024 · 0 comments
Assignees

Comments

@FarSeeing
Copy link

Preflight Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for a feature request that matches the one I want to file, without success.

You must agree to search and the code of conduct. You must fill in this entire template. If you delete part/all or miss parts out your issue will be closed.

Describe the bug
When closing a main window several times the confirmation modal is shown multiple times.

To Reproduce
Steps to reproduce the behavior:

  1. Open the app
  2. Open or create a new diagram
  3. Modify that diagram
  4. Close the main window by clicking the close button several times
  5. If you're the fastest hand on the Wild West fast enough - the confirmation modal would be shown multiple times

Expected behavior
Confirmation modal is shown once

Screenshots
If applicable, add screenshots to help explain your problem.

draw.io version (In the Help->About menu of the draw.io editor):

  • draw.io version 24.7.17

Desktop (please complete the following information):

  • OS: Windows, MacOS, Linux...

Additional context
There is a check to display modal once by adding a unique id to each close handler but it doesn't work.
The lifecycle is close event - isModified-result - another close - another isModified-result so this unique id passes the check.

A proposal solution
index 76fa3e8..a59fa7f 100644
--- a/src/main/electron.js
+++ b/src/main/electron.js
@@ -269,11 +269,13 @@ function createWindow (opt = {})
                mainWindow.webContents.send('resize')
        });
 
-       let uniqueIsModifiedId;
+       let isModifiedInProgress;
 
        ipcMain.on('isModified-result', async (e, data) =>
        {
-               if (!validateSender(e.senderFrame) || uniqueIsModifiedId != data.uniqueId) return null;
+               if (!validateSender(e.senderFrame)) return null;
+
+               isModifiedInProgress = true;
 
                if (data.isModified)
                {
@@ -316,23 +318,29 @@ function createWindow (opt = {})
                {
                        mainWindow.destroy();
                }
+
+               isModifiedInProgress = false;
        });
 
        mainWindow.on('close', (event) =>
        {
-               uniqueIsModifiedId = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
-
                if (__DEV__)
                {
                        const index = windowsRegistry.indexOf(mainWindow)
-                       console.log('Window on close', index, uniqueIsModifiedId)
+                       console.log('Window on close', index, isModifiedInProgress)
+               }
+
+               if (isModifiedInProgress)
+               {
+                       event.preventDefault();
+                       return;
                }
 
                const contents = mainWindow.webContents
 
                if (contents != null)
                {
-                       contents.send('isModified', uniqueI
- sModifiedId);
+                       contents.send('isModified');
                        event.preventDefault();
                }

And a sidenote that the close even handler should probably handle a case when window contents have not loaded the JS (because of a CSP error maybe) and thus cannot reply with a isModified-result event.

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

No branches or pull requests

2 participants