RFR: 6531: Make the flame chart view render labels on windows
Marcus Hirt
hirt at openjdk.java.net
Thu Jan 23 23:23:27 UTC 2020
On Thu, 23 Jan 2020 15:35:30 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:
>> The comment in the upstream d3-flame-graph suggests that there is no solution and unless someone comes up with one, nothing will change. Your commits in the fork seem to be a solution. Could that be proposed upstream? Does it introduce issues outside of IE?
>
>> The comment in the upstream d3-flame-graph suggests that there is no solution and unless someone comes up with one, nothing will change. Your commits in the fork seem to be a solution. Could that be proposed upstream?
>
> I'll post back on the original issue to see what they think. It looks like they start using ES6 syntax later on, which would explain why versions `> 2.0.2` don't work on IE, so inclusion of this work may be better suited for a branch of a prior version (2.0.2-ie?).
>
>> Does it introduce issues outside of IE?
>
> Outside of IE I didn't encounter any problems (but I didn't check much more than seeing what it looked like if used in Linux for JMC), although with regards to this patch and JMC the modified JS is only used in Windows environments.
>
> There is a bit more computation in the fork - the text on the labels need to be trimmed for text-overflow instead of having the css format it. From what I understand, elements within `<svg>` tags all belong to the svg namespace, but putting elements inside a `<foreignObject>` [0] allows them to be recognized as being in the HTML namespace. So in the original code the text on the labels be treated as HTML and formatted by the css, but `svg text` doesn't appear to be formatted the same way, so there's an added function in the IE fork [1] that trims the text instead.
>
> [0] https://www.w3.org/TR/SVG2/embedded.html#ForeignObjectElement
> [1] https://github.com/aptmac/d3-flame-graph/blob/internet-explorer-compatibility/dist/d3-flamegraph-ie.js#L5242
To me it's a bit scary (as in possibly maintenance heavy) to make a fork of the d3-library, rather than investing in making the SWT browser component work with Edge or chromium. This could potentially be a stop gap measure though. @mirage22, would this complicate anything you were planning on doing with the flame view?
-------------
PR: https://git.openjdk.java.net/jmc/pull/41
More information about the jmc-dev
mailing list