Skip to content

GUI Support for Enabling/Disabling Native Messaging Support Per Browser #1088

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

eajaykumar
Copy link

Summary

This PR adds a new Browser Integration tab in the Persepolis settings, giving users control over which browsers are integrated via native messaging.

Features:

  1. Users can now explicitly select browsers (Firefox, LibreWolf, Chrome, etc.) to integrate with Persepolis.
  2. When a browser is unselected, its corresponding native messaging manifest is automatically removed.
  3. Manifests are no longer created unnecessarily. Integration is now opt-in.
  4. The list of browsers is shown even if not installed (but auto-check only if installed).
  5. GUI checkboxes reflect real integration status.
  6. Custom manifest path is supported.
  • Changes made to: initialization.py, setting_ui.py, setting.py, browser_integration.py

Tested on Linux (LibreWolf, Firefox). Requires clean config reset for testing.

@eajaykumar
Copy link
Author

@alireza-amirsamimi I've done my best to keep the code clean, modular. However, feel free to edit, simplify, or remove any parts of the code if you find something unnecessary or if it can be done better.
Thanks with this i can say any custom Browser and officials ones will work smoothly with persepolis

@alireza-amirsamimi
Copy link
Member

@eajaykumar
Thank you. I'll review and accept your pull request soon 👍

@guest75582332
Copy link

guest75582332 commented Jul 22, 2025

When testing this branch on Windows, I encountered the following errors:

Failed to remove manifest for librewolf: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for firefox: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for vivaldi: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for opera: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for chrome: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for brave: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for chromium: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType

It seems the issue is that shutil.which() fails to find any browsers on Windows, as they are not typically added to the system PATH. This results in enabled_browsers being an empty list.

Consequently, remove_manifests_for_browsers is called for all all_known_browsers, but since get_native_message_folder doesn't have a Windows implementation, it returns None, leading to the TypeError: path should be string... not NoneType.

This suggests that relying on shutil.which() for browser detection is not enough for cross-platform support. Maybe we could using a more specialized library like browsers to handle this?

@eajaykumar
Copy link
Author

eajaykumar commented Jul 22, 2025

@guest75582332 ,its good way to use cross-platform library browsers ,thanks for testing and recommendation as i don't have windows or mac so i didn't test with those platforms ,i think its a one way shot to support all platforms now with this library i will try and implement it

@eajaykumar
Copy link
Author

i checked the library its only detecting official browsers not custom browsers. can we fork browsers library to support well know custom browsers. i need final call from @alireza-amirsamimi to make it happen

@alireza-amirsamimi
Copy link
Member

alireza-amirsamimi commented Jul 22, 2025

Hello,

I have read your code. Thank you very much for your efforts.

My concern is that the code should work across all operating systems and, of course, not be overly complex so that we can maintain it easily in the future.

I believe it is a good strategy for Persepolis to create configuration files only for Chrome, Chromium, and Firefox. Other common browsers(Opera, Brave, Vivaldi, Libre Wolf) can be selected by the user in the settings window if needed.

We must acknowledge that there are many browsers, and their number is increasing every day, but in reality, users do not use them as their primary browser and there aren't many users for them. I think there is no reason to complicate the code for these browsers. What I mean is that there is no need for a custom path section in the settings window.

If users of these browsers are Persepolis users, they will likely either submit a pull request or open an issue, and we can add the required browser to the code in the future.

@eajaykumar
Copy link
Author

OK its a good strategy so we remove custom manifest path and Only Firefox, Chrome, and Chromium are enabled by default. Users have Options can toggle additional browsers (LibreWolf, Brave, etc.) via the GUI. and we can add support to other browsers per request of users. so no forking of browsers library is required but should we add browsers library for cross platform support or do we hard-code those three default browsers for detecting browser installation.
and also one suggestion is that we need to inform users about browser integration to those who use other custom browsers that wont automatically work by default maybe we should add a line in web extension to inform that there is a browser integration tab that exists to enable them

@guest75582332
Copy link

One of the original intentions behind proposing a custom path option was to support non-standard variants (e.g., Chrome Beta, Chrome Dev, Chrome Canary). In my opinion, you might not want to explicitly support these variants. That said, since all code maintenance ultimately falls on the maintainer, it's still workable without a custom path option—copying Chrome's manifest file to the appropriate location still works. As long as there's a way to clean up useless manifest files, I think that should be sufficient.

@eajaykumar
Copy link
Author

@guest75582332 You're right copying Chrome's manifest manually for other variants works just fine. keeping the implementation simple, and focusing on core browsers like Firefox, Chrome, and Chromium, makes long-term maintenance easier. unused manifest files are properly cleaned up while user unchecks the browser, so users won't be left with unnecessary configurations.

@eajaykumar
Copy link
Author

i just need a final go and advice from @alireza-amirsamimi to add browsers library to use as dependency and also its very light weight as i saw the code. i will remove custom manifest path and only Firefox, Chrome, and Chromium are enabled defaults.

@alireza-amirsamimi
Copy link
Member

i just need a final go and advice from @alireza-amirsamimi to add browsers library to use as dependency and also its very light weight as i saw the code. i will remove custom manifest path and only Firefox, Chrome, and Chromium are enabled defaults.

What do you mean by the browser library?

@eajaykumar
Copy link
Author

for detecting installed browsers in Linux, macs, windows. browsers
with this we can only toggle the check for installed default browsers or we have one option we can hard code enable them

@alireza-amirsamimi
Copy link
Member

@eajaykumar
We can only use libraries that are well-known and are available in the official repositories of all Linux distributions and FreeBSD, such as requests, urllib3, psutil, ... . Otherwise, those who maintain the Perspolis package in the distributions' repositories will not be able to update it.

@guest75582332
Copy link

I checked the status of the pybrowsers library on Repology, it is not a widely packaged library, which would certainly complicate things for downstream maintainers.

https://repology.org/project/pybrowsers/versions

Given this, perhaps the best approach for now is to implement the browser detection logic internally. We could hard-code the necessary checks for the primary targets (Chrome, Chromium, and Firefox) across Windows, macOS, and Linux.

@eajaykumar
Copy link
Author

@guest75582332 can you test it and update result please ,i added support for windows to detect installed browsers

@alireza-amirsamimi
Copy link
Member

So we cannot use the browser library because it is not available in the repositories of Arch Linux, Fedora, openSUSE, FreeBSD, and so on. My ten years of experience in software development shows that package maintainers are reluctant to add a new package that is not in the official distribution repositories, as the process of adding and accepting a package in well-known distributions involves many steps. This means that if we use this library in the code, it is likely that Persepolis will not be updated in these well-known distributions.

@eajaykumar
Copy link
Author

eajaykumar commented Jul 24, 2025

yea i too recognize it ,now i dropped it and hard coded it to streamline every step smoothly without using browsers library, you can check the code

@guest75582332
Copy link

guest75582332 commented Jul 25, 2025

This does fix the browser detection issue. However, remove_manifests_for_browsers still isn’t correctly implemented for non-Linux platforms, which leads to the following errors:

Failed to remove manifest for librewolf: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for brave: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for chromium: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for opera: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for vivaldi: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType
Failed to remove manifest for firefox: _path_exists: path should be string, bytes, os.PathLike or integer, not NoneType

@eajaykumar
Copy link
Author

eajaykumar commented Jul 25, 2025

@guest75582332 i will install virtual windows 10 and will try myself thanks. so its showing default browser check boxes and can you check in browsers folders if manifests generated properly

@guest75582332
Copy link

def remove_manifests_for_browsers(browsers, custom_path=None):
    for browser in browsers:
        try:
            if browser.strip() == "":
                continue
            path = get_manifest_path_for_browser(browser, custom_path)
            if os.path.exists(path):
                os.remove(path)
                print(f"Removed manifest for {browser}: {path}")
        except Exception as e:
            print(f"Failed to remove manifest for {browser}: {e}")

I think the reason for the error is that the current implementation of remove_manifests_for_browsers relies on the file paths returned by get_manifest_path_for_browser, which is currently only implemented for Linux:

if os_type == OS.LINUX: 
 if browser == BROWSER. CHROMIUM: 
 return os.path.join(home_address, '.config/chromium/NativeMessagingHosts') 
 elif browser == BROWSER.CHROME: 
 return os.path.join(home_address, '.config/google-chrome/NativeMessagingHosts') 
 elif browser == BROWSER.FIREFOX: 
 return os.path. join(home_address, '.mozilla/native-messaging-hosts') 
 elif browser == BROWSER.VIVALDI: 
 return os.path.join(home_address, '.config/vivaldi/NativeMessagingHosts') 
 elif browser == BROWSER.OPERA: 
 return os.path.join(home_address, '. config/opera/NativeMessagingHosts') 
 elif browser == BROWSER.BRAVE: 
 return os.path.join(home_address, '.config/BraveSoftware/Brave-Browser/NativeMessagingHosts') 
 elif browser == "librewolf": 
 return os.path.join(home_address, '.librewolf/native-messaging-hosts') 

@eajaykumar
Copy link
Author

@guest75582332 yea i got it that's why am installing windows and checking all browsers paths to implement and make sure everything work properly

@eajaykumar
Copy link
Author

i got it working in windows now ,am just refactoring code and will update it

@eajaykumar
Copy link
Author

@alireza-amirsamimi @guest75582332 i encountered conflict on Windows, all Chromium-based browsers (Chrome, Brave, Chromium,etc.) shared the same native messaging host name and manifest. This caused a conflict where disabled browsers could still capture downloads, because Windows uses the same registry key for all as the extension connects to the same host name and extension id.
now i added unique native messaging host names for each Chromium-based browser like com.persepolis.chrome, com.persepolis.brave etc. updated the manifest generation and registry entries so that each browser uses its own native host. this prevents disabled Chromium browsers from capturing downloads when not selected in settings.
to make this work i need to edit code in persepolis web extension to detect its own host name

function getNativeHostName() {
    const browser = detectBrowser();
    if (DEBUG) {
        console.log("Detected browser:", browser);
    }
    switch (browser) {
        case "chrome":
            return "com.persepolis.chrome";
        case "brave":
            return "com.persepolis.brave";
        case "edge":
            return "com.persepolis.edge";
        case "vivaldi":
            return "com.persepolis.vivaldi";
        default:
            return "com.persepolis.pdmchromewrapper";
    }
}

with this changes only the enabled browser’s manifest works and every browser has unique host name and we can add a fallback to for older version with this com.persepolis.pdmchromewrapper, with this changes its efficiently captures links

@alireza-amirsamimi
Copy link
Member

@eajaykumar
Thank you for the effort you put in. Let me see if I can find an easier solution.

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