feat: add confirmation for opening external apps

ericyzhu commented 1 week ago

follow-reviewer-bot[bot] commented 1 week ago

Thank you for your contribution. We will review it promptly.

vercel[bot] commented 1 week ago

@ericyzhu is attempting to deploy a commit to the RSS3 Team on Vercel.

A member of the Team first needs to authorize it.

follow-reviewer-bot[bot] commented 1 week ago

Suggested PR Title:

feat: add external URL confirmation dialog for navigation

Change Summary: Enhanced window navigation with external URL confirmation dialog and added localization for dialog messages.

Code Review:

File Path: apps/main/src/window.ts, Line 110-124: The webContents.on("will-navigate") event handler's logic to check the protocol is appropriate for avoiding potential security risks; however, it currently does not account for http and https URLs. The user will not be prompted and the e.preventDefault() will prevent navigation for these protocols. Ensure that desired behavior is allowed for http and https URLs:
```
if (protocol === "http:" || protocol === "https:" || protocol === "follow:") {
// Handle internal navigation for these protocols if needed
return;
}
```
File Path: apps/main/src/window.ts, Line 110-112: You should also unregister or clean up event listeners to prevent memory leaks. Ensure the listener is removed when the BrowserWindow is closed:
```
window.on("closed", () => {
webContents.removeAllListeners("will-navigate");
});
```

These changes are necessary to ensure the code is robust, secure, and free from potential memory leaks.

Aside from the above points, everything else looks good!

If you make these changes, it should address the identified issues and enhance the overall quality of the code.

Thus, ensuring a thorough and well-rounded review.

Innei commented 6 days ago

@ericyzhu I modified some logic. Everything is working well.

RSSNext / Follow