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

Fix GitLabClient.js get wrong draw.io host path when window.location href contains "/" #805

Closed
wants to merge 2 commits into from

Conversation

nic-6443
Copy link

relate to #493 (comment). For example, use
https://MyDraw.io/?gitlab=https://MyGitLab.com&gitlab-id=AppId to access my self host draw.io server, GitlabClient.js generate rediretUrl query parameter will be https://MyDraw.io/?gitlab=https://gitlab.html, correct should be https://MyDraw.io/gitlab.html.

@davidjgraph
Copy link
Collaborator

We can't merge this now since you've written your custom values to the diff.

If you want to set variable, use preConfig.js to write them without having to rebuild.

@nic-6443
Copy link
Author

We can't merge this now since you've written your custom values to the diff.

If you want to set variable, use preConfig.js to write them without having to rebuild.

Sorry, the custom values setting was caused by my temporary test, I forgot it will be updated to this PR.
You can reconsider whether shoude accept the first commit for gitlab redirect url fix, if necessary I can reopen a new PR.

@davidjgraph
Copy link
Collaborator

What's an example URL where your first change fixes it?

@davidjgraph
Copy link
Collaborator

Ah sorry, you posted in top post, a lot of issues, reading too fast :).

@davidjgraph
Copy link
Collaborator

OK, the URL parameters must be URI encoded. If we're not decoded them, that's actually a different bug. Try again with properly encoded URL parameters

@nic-6443
Copy link
Author

nic-6443 commented May 14, 2020

You are right, without URLencoding, href.lastIndexOf ('/')) may not find the correct route for current Draw.io.

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.

None yet

2 participants