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.

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

Links in graph missing #209

Closed
Profpatsch opened this issue Sep 19, 2015 · 16 comments
Closed

Links in graph missing #209

Profpatsch opened this issue Sep 19, 2015 · 16 comments
Milestone
0.5.2

Comments

@Profpatsch
Copy link 

          graph TD;
            A-->B;
            B-->C;
            C-->D;
            A-->D;

gives
screenshot
@lmartel
Copy link

lmartel commented Sep 20, 2015

+1

@Daijobou
Copy link

With your graph, I see only the lines and not the arrows. ;)

@Profpatsch
Copy link
Author

Yeah, I realized: Just use graphviz, duh (aka: it’s an already solved problem).

@knsv
Copy link
Collaborator

knsv commented Sep 21, 2015

Have you included the mermaid.css? If not that would probably solve your issue. Hope it helps.

If this is rendered via the CLI the latest unreleased version includes the stylesheet.

@Daijobou
Copy link

Arrows not need the CSS file. Here is a strange problem with BASE html-tag. Please create a blank html file and drop this in it:

<!DOCTYPE html>
<html>
<head><base href="XXX">
<meta charset="utf-8">
<title></title>
<script src="https://rawgit.com/knsv/mermaid/master/dist/mermaid.min.js"></script>
</head><body>
<script>mermaid.initialize();</script>
<div class="mermaid">
graph LR;
    A-->B;
    B-->C;
</div>
</body></html>

You will not see any arrows. BUT now remove "XXX" in href of BASE tag and its will work. Strange, or?

@knsv
Copy link
Collaborator

knsv commented Sep 23, 2015

If you look in the svg code for an arrow it will make more sense:

<g class="edgePath" style="opacity: 1;">
<path class="path" d="M44,55L44,80L44,105" marker-end="url(#arrowhead7)" style="fill:none"></path><defs>
   <marker id="arrowhead7" viewBox="0 0 10 10" refX="9" refY="5" markerUnits="strokeWidth" markerWidth="8" markerHeight="6" orient="auto">
      <path d="M 0 0 L 10 5 L 0 10 z" style="fill: #333"></path>
   </marker>
</defs>
</g>

Look at the marker end part where it is actually a url pointing to the definition. Thats why the base path change messes it up.

@Daijobou
Copy link

Yes. I found this stackoverflow question for this topic:
http://stackoverflow.com/questions/18259032/using-base-tag-on-a-page-that-contains-svg-marker-elements-fails-to-render-marke
Its possible to add location.href directly in mermaid for solve this problem?
"location.href" without the fragment part. => http://domain.tld/folder/file.html#fragment ;)

@wch
Copy link

wch commented Sep 25, 2015

I believe it could be fixed by replacing this line https://github.com/knsv/mermaid/blob/fcb2af7/src/diagrams/sequenceDiagram/sequenceRenderer.js#L231:

        line.attr("marker-end", "url(#arrowhead)");

with:

        line.attr("marker-end", "url(" + window.location.href + "#arrowhead)");

And the same for the #crosshead line four lines below.

That, at least, is the fix I made for another project (which worked): https://github.com/vega/vega/pull/213/files

@knsv
Copy link
Collaborator

knsv commented Sep 26, 2015

Excellent!
That works for sequenceDiagrams. I can apply the same approach for flowcharts but through adjusting the generated code from dagre-d3.

Thanks!

@knsv
Copy link
Collaborator

knsv commented Sep 26, 2015

Small issue. The url wont work if the actual url contains a hash section as in http://a.b.c/d#e

Using

window.location.protocol+'//'+window.location.host+window.location.pathname

instead of location.href solves this though so all is well!

knsv added a commit that referenced this issue Sep 26, 2015
@knsv
Fix for issue #209, missing links when baste tag is used
fb94aaa 
Fix for issue #195, text wrap in sequence diagrams drops last word
Documentation
@knsv knsv added the Fixed label Sep 26, 2015
@knsv knsv added this to the 0.5.2 milestone Oct 4, 2015
@knsv
Copy link
Collaborator

knsv commented Oct 4, 2015

Fix for this issue was released in mermaid 0.5.2. Will close this now, let me know if you issues still remain.

@knsv knsv closed this as completed Oct 4, 2015
@wch wch mentioned this issue Oct 5, 2015
Closed
@wch
Copy link

wch commented Oct 23, 2015

@knsv, thanks for implementing the fix! However, I think the solution still isn't quite right -- if your URL is something like http://a.b.c/d?e#f, then it returns "http://a.b.c/d", but the ?e should be in there also.

@wch
Copy link

wch commented Oct 23, 2015

Also, there's the port number to consider... I found a couple of relevant questions on SO:
http://stackoverflow.com/questions/5818269/javascript-window-location-href-without-hash
http://stackoverflow.com/questions/15193562/javascript-document-url-without-the-anchor-hash-portion

@knsv
Copy link
Collaborator

knsv commented Oct 23, 2015

You are right. It is easy to underestimate urls. Have you found instances where this does not work?

In the end the code that handles this is the following:

window.location.protocol+'//'+window.location.host+window.location.pathname +window.location.search;

The port is included with window.location.host and the hash should be excluded otherwise there is two hashes in the url.

My test url:
http://127.0.0.1:8080/advancedBasteTag(.html?apa=1(a)2#as

gives:
http://127.0.0.1:8080/advancedBasteTag\(.html?apa=1\(a\)2#arrowhead

which renders the arrow heads.

@wch
Copy link

wch commented Oct 23, 2015

That looks good to me, but I'm not an expert on this topic. This issue came to mind again because I was doing some SVG stuff in a project. I ended up using window.location.href.split("#")[0], but I don't know if that's any better or worse than your solution.

@knsv
Copy link
Collaborator

knsv commented Oct 24, 2015

Sounds good! Thanks for keeping mermaid in mind!

mgenereu pushed a commit to mgenereu/mermaid that referenced this issue Jun 25, 2022
@sidharthv96
Merge pull request mermaid-js#209 from Morreski/develop
be6931d 
Mitigate XSS vulnerability and allow img tags to be correctly handled in output SVG.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.5.2
Development

No branches or pull requests
5 participants
@wch @Daijobou @lmartel @Profpatsch @knsv