[Rev 04] RFR: 6531: Make the flame chart view render labels on windows
Alex Macdonald
aptmac at openjdk.java.net
Mon Mar 9 16:36:14 UTC 2020
> 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
Alex Macdonald has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev
excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since
the last revision:
- update the removeSpecialCharacters function to be compliant with IE
- fix spacing of <execution> tags in the pom.xml
- rename page.template to template.html for compatibility with IDE syntax highlighting
- 6531: Make the flame chart view render labels on windows
-------------
Changes:
- all: https://git.openjdk.java.net/jmc/pull/41/files
- new: https://git.openjdk.java.net/jmc/pull/41/files/885913f8..d2f5d5a2
Webrevs:
- full: https://webrevs.openjdk.java.net/jmc/41/webrev.04
- incr: https://webrevs.openjdk.java.net/jmc/41/webrev.03-04
Stats: 652 lines in 31 files changed: 554 ins; 15 del; 83 mod
Patch: https://git.openjdk.java.net/jmc/pull/41.diff
Fetch: git fetch https://git.openjdk.java.net/jmc pull/41/head:pull/41
PR: https://git.openjdk.java.net/jmc/pull/41
More information about the jmc-dev
mailing list