RFR: 6531: Make the flame chart view render labels on windows
Jie Kang
jkang at openjdk.java.net
Fri Jan 24 16:55:09 UTC 2020
On Thu, 23 Jan 2020 23:23:03 GMT, Marcus Hirt <hirt 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?
>>
>> 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?
While the guard to only use this on Windows does mitigate impact of this patch, I also am concerned with maintenance of a fork of the d3-flame-graph. I would rather see the upstream updated, or as Marcus stated, the SWT browser updated to use an alternative (newer) browser.
-------------
PR: https://git.openjdk.java.net/jmc/pull/41
More information about the jmc-dev
mailing list