Skip to content

Exposing Whitelisted Protocols to User Settings #1401

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Exposing Whitelisted Protocols to User Settings #1401

wants to merge 1 commit into from

Conversation

@jonassorgenfrei
Copy link

What's this PR do?

It exposes the whitelisted protocols list so users can add additional protocols that should not use the link wrapper.
Adresses Issue: #1284

You have tested this PR on:

  • Windows
  • Linux/Ubuntu

This was referenced Dec 27, 2024
Open
Closed
@jonassorgenfrei jonassorgenfrei changed the title implementing whitelisted protocols to allow configuring linkifiers in… Exposing Whitelisted Protocols to User Settings Dec 27, 2024
@alya
Copy link
Collaborator

alya commented Dec 27, 2024

Thanks! Could you please update your commit message to match the commit style guidelines?

@andersk
Copy link
Member

andersk commented Dec 27, 2024

Do not close and reopen multiple pull requests for the same thing, even to fix the commit structure; you can always fix the commit structure by rebasing and force-pushing your existing branch.

@jonassorgenfrei
Copy link
Author

I adjusted the commit message to match the style guide.
Yeah i messed i my repo on the first PR by directly implementing it on the main branch and by messing up my rebasing. Next time i know better :)

@jonassorgenfrei
Copy link
Author

Is there anything left to do on this PR?

@jonassorgenfrei jonassorgenfrei force-pushed the WhitelistedProtocols branch 2 times, most recently from 87f4cff to f56c2d0 Compare July 16, 2025 22:04
@jonassorgenfrei
Copy link
Author

@andersk @timabbott can you take a look if there's anything missing here that avoid a merge?

@timabbott
Copy link
Member

I think we'd need documentation for how users can decide which protocols they can safely add there.

@jonassorgenfrei
Copy link
Author

jonassorgenfrei commented Jul 19, 2025 •
edited
Loading

@timabbott

I think we'd need documentation for how users can decide which protocols they can safely add there.

Totally makes sense, i'm just unsure where to document the setting.json 's new key.
I could put it in the docs but would probably have to add a new .md file like desktop-app-settings.md?
On Zulip Help i didn't find a section about the setting.json.

Could you point me to where i shall add the information then i'm happy to add it there too

@timabbott
Copy link
Member

A new file seems fine. I'd call it customize-link-protocols.md

@zulipbot zulipbot added size: M and removed size: S labels Jul 21, 2025
@jonassorgenfrei
Copy link
Author

Allright i added a customize-link-protocols.md to docs/howto :)
Hope that's fine.

@zulipbot zulipbot added size: XL and removed size: M labels Aug 14, 2025
@zulipbot zulipbot added size: XL and removed size: M labels Aug 14, 2025
@zulipbot zulipbot added size: M and removed size: XL labels Aug 14, 2025
@zulipbot zulipbot added size: XL and removed size: M labels Aug 14, 2025
@zulipbot zulipbot added size: M and removed size: XL labels Aug 14, 2025
@zulipbot zulipbot added size: L and removed size: M labels Aug 14, 2025
@zulipbot zulipbot added size: XS and removed size: L labels Aug 14, 2025
@jonassorgenfrei
link_utils: exposing whitelisted protocols to user settings
0396e6c 
Adding config option to set protocols in the config that are
whitelisted to be opened directly.
The behaviour is documented in docs\howto\customize-link-protocols.md.
@zulipbot zulipbot added size: M and removed size: XS labels Aug 14, 2025
@jonassorgenfrei
Copy link
Author

@timabbott could you may be take another look on the PR?

Everything should be there now, would be cool if we can merge it in main,
as i often had to fix merge conflicts with the latest changes to keep this pr uptodate.

shubham-padia
shubham-padia reviewed Oct 24, 2025
View reviewed changes
Copy link
Member

@shubham-padia shubham-padia left a comment

Choose a reason for hiding this comment

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

This is great work! Tested and it works fine. Requested some changes for comments.

app/common/link-util.ts
import {Html, html} from "./html.ts";
import * as t from "./translation-util.ts";

/* Fetches the current protocolLaunchers from settings.json */
Copy link
Member

Choose a reason for hiding this comment

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

protocolLaunchers doesn't seem like a noun. I don't think this comment is needed, since it is clear from using ConfigUtil.getConfigItem( that we are fetching from settings.json

docs/howto/customize-link-protocols.md
By default, the following protocols are whitelisted:

```
http https mailto tel sip
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can separate this using a comma?

docs/howto/customize-link-protocols.md

It is possible to customize the list of whitelisted protocols by editing the `settings.json` file located at: `userdata/Zulip/config/settings.json`

To add or modify the list, the `whitelistedProtocols` key can be updated. For example:
Copy link
Member

Choose a reason for hiding this comment

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

To add or modify -> To modify

docs/howto/customize-link-protocols.md

Links using these protocols are opened directly by the system.

All other protocols are considered potentially unsafe and are therefore opened indirectly—through a local HTML file—in your default web browser.
Copy link
Member

Choose a reason for hiding this comment

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

file-in -> file in

@shubham-padia shubham-padia self-assigned this Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@shubham-padia shubham-padia shubham-padia left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@shubham-padia shubham-padia

Labels

size: M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jonassorgenfrei @alya @andersk @timabbott @shubham-padia @zulipbot