sverweij / mscgen_js

text => sequence charts
https://mscgen.js.org
GNU General Public License v3.0
206 stars 25 forks source link

Links do not work for mscgen.js in chrome and IE #244

Closed daledude closed 7 years ago

daledude commented 7 years ago

The below example from the documentation produces links but clicking them does nothing.

https://mscgen.js.org/tutorial.html#url-id-idurl

sverweij commented 7 years ago

@daledude thanks for raising this.

This issue seems to occur in Chrome only - in Firefox & Safari links in the svg still work as expected) - I'll dive into it.

=> Is Chrome the browser you're using?

daledude commented 7 years ago

I should have put the browsers I tested. Chrome 57.0.2987.110 and MS Edge 38.14393.0.0. Slightly off topic. It would be useful if the link didn't open in a new window since I use anchors to scroll to the id's on the current page outside the svg.

As a test the below works as intended.

                <svg
                     xmlns="http://www.w3.org/2000/svg"
                     xmlns:xlink="http://www.w3.org/1999/xlink"
                     width="150" height="150">
                    <a xlink:href="#SBC-LosAngeles-20"><text x="25" y="25">link</text></a>
                    <a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="#SBC-LosAngeles-20" xlink:title="#SBC-LosAngeles-20" xlink:show="new"><text x="55" y="55">|0|Trying</text></a>
                </svg>
daledude commented 7 years ago

Or an onClick would be useful.

sverweij commented 7 years ago

Thanks for the browser versions - I've confirmed on IE11 as well.

Weirdly:

onClick

I'll look into that as well. What would you use it for?

open in 'new window'

About that open in 'new window' (xlink:show): ouch :grimacing: I'll remove that a.s.a.p. Seemed like a good idea back in the day, but I agree that it's actually super annoying.

sverweij commented 7 years ago

@daledude a small update:

daledude commented 7 years ago

Thanks!

I think it's a problem of these browsers not handling nested xlinks. A simple test below shows the links outside the "use" tags work but linked from the "use" do not. Or there is something overriding their events. I haven't been able to find anything specific to this due the words needed to search google for it!

<html>
<body>
<svg width="80" height="80" xmlns="http://www.w3.org/2000/svg"
     xmlns:xlink="http://www.w3.org/1999/xlink">
    <style>
        .classA {
            fill: red;
        }
    </style>
    <defs>
        <g id="Port">
            <circle style="fill: inherit;" r="10"/>
            <text>
                <a xlink:href="https://www.wikipedia.org">
                    <tspan>Link</tspan>
                </a>
            </text>
        </g>
    </defs>

    <text y="15">
        <a xlink:href="http://www.google.com">
            <tspan>Google</tspan>
        </a>
    </text>
    <use x="50" y="10" xlink:href="#Port"/>
    <text y="35">
        <a xlink:href="http://www.facebook.com">
            <tspan>Facebook</tspan>
        </a>
    </text>
    <use x="50" y="30" xlink:href="#Port" class="classA"/>
    <text y="55">
        <a xlink:href="http://www.twitter.com">
            <tspan>Twitter</tspan>
        </a>
    </text>
    <use x="50" y="50" xlink:href="#Port" style="fill: blue;"/>
</svg>
</body>
</html>
daledude commented 7 years ago

The reason for the onClick request is that's what I currently use in mscgen generated svg's. I "hack" the svg with javascript to replace the links so it will call my javascript function. My javascript function in turn smooth scrolls to the anchor on the page.

At the moment it's useless since it doesn't work within a "use" tag either. But, once the problem with the link is resolved I can use the #id syntax instead.

Thanks for taking the time.

daledude commented 7 years ago

Maybe this part of https://www.w3.org/TR/SVG2/linking.html#Links is relevant. Not sure if it just means nested a elements or any element that has an href.

Links may not be nested; if an ‘a’ element is a descendent of another hyperlink element (whether in the SVG namespace or another namespace), user agents must ignore its href attribute and treat it as inactive. The invalid ‘a’ element must still be rendered as a generic container element.

sverweij commented 7 years ago

The problem for chrome & IE definitely lies in the construction with use elements (as you also point out). They were trouble when the title attribute was introduced as well.

I have a working proof of concept that wraps the use in an a - a solution similar to what was used with the title attribute. It seems to work quite well, buIt needs polishing. It might have drawbacks I don't see right now because I need to get some sleep.

The permanent solution is probably to restructure the svg to not use use anymore on things that can have an a attribute in them (almost anything 😬). I've been playing with that idea for some time but didn't have enough incentive to actually do it.

Strangely, both chrome & IE do recognise the links, and are able to follow them (left-click => open link in new tab/ open link in new window work fine for both). Bug or not I'm not going to hold my breath for Google & Microsoft to fix it.

B.t.w.: Thanks for your prompt feedback & helpful additions. I really appreciate that.

daledude commented 7 years ago

I was going to propose just in-lining the "use" sections. You would know better than I the pro's and con's of doing that.

I've spent quite awhile trying to find something related but haven't. The closest was a jquery ticket about attaching click events. https://bugs.jquery.com/ticket/13180

Thanks for the effort you have put in for this!

sverweij commented 7 years ago

Inlining (replacing the uses with the actual stuff currently in the defs) was what I was thinking of in the permanent solution as well. Pros: smaller svg, easier to debug. Con: Quite a refactoring as the rendering code was built on the nice abstraction uses give.

I'll probably do the simple fix first and the refactoring later.

I've tried to parse the w3c document as well - it seems it only forbids nesting real links which totally makes sense (<a href="#one"><a href="#two">where does this link go?</a></a>) Links and uses should be mixable (and are - even in chrome and IE ...)

sverweij commented 7 years ago

The simple fix started to get very hacky, so I'm refactoring the graphics renderer to 'inline' elements. I've done the entities and I like the result. The links work and the rendered svg snippet with the entities is a lot easier to read. I hope I can do the arcs this weekend...

daledude commented 7 years ago

Looking forward to using it. Thanks again for all the effort! Will be nice to replace mscgen and my hacks to its output.

sverweij commented 7 years ago

@daledude Incidentally I've just finished development. If you want to, you can test it - I've deployed it on a feature branch on mscgen.js.org - let me know what you think.

These links will work for a few days - until I've made a release (I hope to do that tomorrow - if there's no regressions)

daledude commented 7 years ago

Almost rick rolled...

Seems to work good. #id scrolls to anchors. http://... loads url.

Off topic...is there a way to customize the Named Styles without modifying the code?

sverweij commented 7 years ago

👍

@ customizing named styles - yes and no

This said, I'm interested in what type of customizations you want to make. Maybe I can make things more customization friendly (and maybe the customizations you have are useful for others as well).

<!DOCTYPE html>
<html>
    <head>
        <meta charset="utf-8">
        <title>mscgen.js style tweaking sample</title>
        <script src="https://mscgen.js.org/mscgen-inpage.js" defer></script>
        <!--
        available entities and classes
        - `svg, line, rect, path, text`
        - `rect`: `.entity .box .rbox .inline_expression .label-text-background
          .bglayer`
        - `line` and `path` (for selfs): `.arc .directional .nondirectional,
          .bidirectional .signal .method .return .callback .lost .emphasised`
        - `line` and `path`: `.box`
        - `line` only: `.comment .inline_expression_divider`
        - `path` only: `.note .abox .inline_expression_label`
        - `text`: `.entity-text .box-text .note-text .rbox-text .abox-text
          .empty-row-text .directional-text .nondirectional-text .bidirectional-text
          .signal-text .method-text .return-text .callback-text .lost-text
          .emphasised-text`
        - `.watermark`
        -->
        <style>
            svg {
                font-family: monospace; /* gets overriden in the svg */
            }
            .entity-text {
                font-family: serif; /* entities currently inherrit these from
                                      `svg`=> .entity-text is more specific
                                      than this inheritance, so this'll be
                                      applied */
            }
            .return-text { /* these'll work */
                font-weight: lighter;
                font-style: italic;
                fill: green;
            }
            .return {
                stroke: green; /* the svg has a  (line. /path.) return class
                                 with stroke:black and that'd override this*/
            }
            .lost-text { /* this will work */
                fill: red;
                font-weight: bold;
            }
            .watermark { /* this will work as well */
                font-family: cursive;
            }
        </style>
    </head>
    <body>
        <script type="text/x-mscgen">
        # OpenId Connect protocol
        # https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.1.3
        msc {
          wordwraparcs="true", watermark="cursive watermark";

          eu [label="end-user"],
          rp [label="relying party"],
          op [label="OpenID provider"];

          eu =>> rp [label="log me in"];
          rp =>> op [label="authentication request"];
          op =>> eu [label="authentication and authorization request"];
          eu >> op [label="authenticate and authorize"];
          op >> rp [label="authentication response"];
          rp =>> op [label="UserInfo request"];
          op >> rp [label="UserInfo response"];
          rp >> eu [label="Hi. You're logged in with {UserInfo.name}"];
          rp -x op [label="lost"];
        }
        </script>
    </body>
</html>
screen shot 2017-03-26 at 12 35 13
sverweij commented 7 years ago

@daledude I've published the changes that should fix the issue for mscgenjs-core, mscgenjs-inpage and mscgenjs-cli to npm, and updated mscgen.js.org and the atom-mscgen-preview package.

Enjoy!

sverweij commented 7 years ago

@daledude I'll close this issue as the changes released in v1.9.10 & v1.9.11 resolve it as far as I can see

If you want to know more about (/ have suggestions to imrpove) customizing styling I'm happy to discuss in a separate issue.

daledude commented 7 years ago

I haven't had a chance to worry about the css yet but will open a new issue when ready.

The new version is working great!