RFR: 6531: Make the flame chart view render labels on windows
Alex Macdonald
aptmac at openjdk.java.net
Thu Jan 23 15:36:20 UTC 2020
On Wed, 22 Jan 2020 22:29:20 GMT, Jie Kang <jkang at openjdk.org> wrote:
>> This PR addresses JMC-6531 [[0]](https://bugs.openjdk.java.net/browse/JMC-6531), in which the flame chart view displays nothing in Windows.
>>
>> tl;dr: the proposed solution uses a fork of d3-flame-graph [[1]](https://github.com/aptmac/d3-flame-graph) which substitutes the incompatible svg element with one that works with Internet Explorer, and modifies the flameviewColoring script to work with an older ECMA version.
>>
>> Screenshot of Windows:
>> 
>>
>> For reference, here's the same view in Linux:
>> 
>>
>> When I had originally taken a look into the SWT Browser and compatibility with Windows, it looked as though we'd be able to use Edge as an embedded browser, which would be nice because it already plays nice with the d3-flame-graph library. After quite a bit of trial and error I was first able to get d3 to work (making simple line charts), and then found that d3-flame-graph version <= 2.0.2 will work. With this older version of d3-flame-graph the coloured labels will be drawn, but no text is shown - regardless of the SWT browser property being set to IE or Edge .. which makes it look like Edge can be used for visiting external webpages, but not local ones (internal browser stays IE). This is because the foreignObject element is incompatible with IE, and this has already been raised as an issue and closed as a "wontfix" by the maintainer on the upstream repo [[2]](https://github.com/spiermar/d3-flame-graph/issues/84).
>>
>> I made a fork of the d3-flame-graph repo [[1]](https://github.com/aptmac/d3-flame-graph), which accomplishes two things:
>> - substitutes `<foreignObject>` for `<text>` [[3]](https://github.com/aptmac/d3-flame-graph/commit/c508e2ab95de2a48f03d655d6e7f306273dcaa12#diff-e866318288d23357a14da878e2435b49L5211)
>> - trims the text in the labels because the ellipsis css doesn't work [[4]](https://github.com/aptmac/d3-flame-graph/commit/c508e2ab95de2a48f03d655d6e7f306273dcaa12#diff-e866318288d23357a14da878e2435b49R5238)
>>
>> This forked version of d3-flame-graph is downloaded by the maven-download-plugin like the other jslibs, and is used if the Environment type is `WINDOWS` as detected in `FlameGraphView.java` [[5]](https://github.com/aptmac/jmc/commit/b64b4349057a67aeeb6b5ef560261cdb616a9d24#diff-9cb007d54dc422cf345941df647ef24cR102).
>>
>> Now that the flame chart view works at this point, I had to edit `flameviewColoring.java` to be compatible with IE. From what I can find, Internet Explorer uses JScript, which in it's latest version supports ECMA 5 [[6]](https://en.wikipedia.org/wiki/JScript#JScript). This means no maps, no named functions, etc.
>>
>> Lastly, and not completely related to this issue, I changed the name of `page.template` to `template.html` because my IDE won't provide syntax highlighting unless the file ends with -.html, and it was a big help here. If it's deemed unecessary I can just drop the associated commit.
>>
>> Let me know what you think!
>>
>> [0] https://bugs.openjdk.java.net/browse/JMC-6531
>> [1] https://github.com/aptmac/d3-flame-graph
>> [2] https://github.com/spiermar/d3-flame-graph/issues/84
>> [3] https://github.com/aptmac/d3-flame-graph/commit/c508e2ab95de2a48f03d655d6e7f306273dcaa12#diff-e866318288d23357a14da878e2435b49L5211
>> [4] https://github.com/aptmac/d3-flame-graph/commit/c508e2ab95de2a48f03d655d6e7f306273dcaa12#diff-e866318288d23357a14da878e2435b49R5238
>> [5] https://github.com/aptmac/jmc/commit/b64b4349057a67aeeb6b5ef560261cdb616a9d24#diff-9cb007d54dc422cf345941df647ef24cR102
>> [6] https://en.wikipedia.org/wiki/JScript#JScript
>
> 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
-------------
PR: https://git.openjdk.java.net/jmc/pull/41
More information about the jmc-dev
mailing list