Skip to content

Conversation

@deepak20001
Copy link

Summary

Implements support for relative URLs in display_icon field for external authentication methods, as requested in #1969.

Changes

  • Modified lib/widgets/login.dart to resolve relative URLs against realmUrl before passing to Image.network()
  • Added unit test in test/widgets/login_test.dart to verify relative URL resolution works correctly

Implementation Details

The fix checks if displayIcon starts with http:// or https://. If not, it resolves the relative URL against realmUrl using Uri.resolve(), matching the pattern used for loginUrl resolution at line 359.

Example:

  • Relative URL: /static/images/google-icon.png
  • Resolved to: https://chat.zulip.org/static/images/google-icon.png

Testing

  • ✅ Unit test added and passing: Verifies relative URLs are correctly resolved to absolute URLs
  • ✅ Backward compatibility: Absolute URLs still work as before (verified by existing test)
  • ⚠️ Manual testing: I wasn't able to set up a test Zulip server to manually verify this. The unit test covers the logic, but would appreciate help from maintainers or other contributors to verify the manual testing requirement mentioned in the issue.

Related

Fixes #1969

Fixes zulip#1969

- Resolve relative displayIcon URLs against realmUrl before passing to Image.network()
- Add unit test to verify relative URL resolution
- Maintain backward compatibility with absolute URLs

The fix checks if displayIcon starts with http:// or https://. If not,
it resolves the relative URL using Uri.resolve(), matching the pattern
used for loginUrl resolution.
@gnprice
Copy link
Member

gnprice commented Nov 8, 2025

I wasn't able to set up a test Zulip server to manually verify this.

Have you looked at the "Getting help" section in the Zulip contributing guide?

In general, this PR description reads like it might if you had an LLM write it for you. That generally won't produce something that meets our standards for communicating clearly. I recommend reading our AI use policy and guidelines:
https://zulip.readthedocs.io/en/latest/contributing/contributing.html#ai-use-policy-and-guidelines

@deepak20001
Copy link
Author

I wasn't able to set up a test Zulip server to manually verify this.

Have you looked at the "Getting help" section in the Zulip contributing guide?

In general, this PR description reads like it might if you had an LLM write it for you. That generally won't produce something that meets our standards for communicating clearly. I recommend reading our AI use policy and guidelines: https://zulip.readthedocs.io/en/latest/contributing/contributing.html#ai-use-policy-and-guidelines

Thanks for the explanation. I’m still new to open-source, and I relied on an AI tool for writing the PR description, which I now understand isn’t acceptable here. I appreciate you taking the time to spell out the policy.

For the manual testing: I tried setting up the dev server, but the Docker image ended up taking most of my disk space and I couldn’t get it running properly. Because of that, I may switch to a smaller or more approachable issue for now while I get more comfortable with the workflow.

Thanks again for the guidance & I’ll keep your AI guidelines in mind in future.

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.

Handle relative links in auth method display icons

2 participants