[PATCH] JMC-5419 : Links in rules do not work
Miro Wengner
miro.wengner at gmail.com
Sat Sep 8 07:55:28 UTC 2018
Hi Marcus and All,
it was my pleasure !
nice spot!
I’m thinking if we update it then we need to care about the new group and we may slightly move from
the statement what you have written ‘we want people to only use strict html subset’.
but you are right I can probably improve the regular expression to reflect some possible deviations from the standard.
Thank you!
Kind Regards,
Miro
> On Sep 7, 2018, at 5:15 PM, Marcus Hirt <marcus.hirt at oracle.com> wrote:
>
>
> Hi Miro,
>
> The only nit is that href (as the matching is written) must be the first
> parameter. You could put another capture group between the a and href. That
> said, since it normally is, and we want people to only use a strict html subset
> in rule results anyways, I think it is fine. Unless someone objects, I'll go
> ahead and push later tonight.
>
> Thanks for the contribution!
>
> Kind regards,
> Marcus
>
> On 2018-09-07, 00:33, "application/pgp-signature Miro Wengner protocol=; micalg=pgp-sha256" <miro.wengner at gmail.com> wrote:
>
>
> Hi Marcus,
> thank you for review and suggestions and help!
> I’ve renamed the variable and method to the following:
>
> String adjustedHtml = adjustAnchorFollowAction(...
>
> and I’ve add the comments to the newly created method : adjustAnchorFollowAction
>
> I hope now is all correct.
> Kind Regards,
> Miro
>
> updated diff:
>
> diff -r aac652534dc7 application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultReportUi.java
> --- a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultReportUi.java Thu Sep 06 20:39:49 2018 +0200
> +++ b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultReportUi.java Fri Sep 07 00:24:09 2018 +0200
> @@ -39,8 +39,11 @@
> import java.util.Collection;
> import java.util.HashMap;
> import java.util.List;
> +import java.util.Map;
> import java.util.concurrent.ConcurrentLinkedQueue;
> import java.util.logging.Level;
> +import java.util.regex.Matcher;
> +import java.util.regex.Pattern;
> import java.util.stream.Collectors;
> import java.util.stream.Stream;
>
> @@ -49,13 +52,18 @@
> import org.eclipse.swt.SWTException;
> import org.eclipse.swt.browser.Browser;
> import org.eclipse.swt.browser.BrowserFunction;
> +import org.eclipse.swt.browser.CloseWindowListener;
> +import org.eclipse.swt.browser.OpenWindowListener;
> import org.eclipse.swt.browser.ProgressAdapter;
> import org.eclipse.swt.browser.ProgressEvent;
> +import org.eclipse.swt.browser.WindowEvent;
> import org.eclipse.swt.graphics.ImageData;
> import org.eclipse.swt.graphics.ImageLoader;
> +import org.eclipse.swt.layout.FillLayout;
> +import org.eclipse.swt.widgets.Display;
> import org.eclipse.swt.widgets.Event;
> import org.eclipse.swt.widgets.Listener;
> -
> +import org.eclipse.swt.widgets.Shell;
> import org.openjdk.jmc.common.IState;
> import org.openjdk.jmc.common.IWritableState;
> import org.openjdk.jmc.flightrecorder.rules.IRule;
> @@ -79,6 +87,8 @@
>
> private static final String OVERVIEW_MAKE_SCALABLE = "overview.makeScalable();"; //$NON-NLS-1$
> private static final String OVERVIEW_UPDATE_PAGE_HEADERS_VISIBILITY = "overview.updatePageHeadersVisibility();"; //$NON-NLS-1$
> + private static final Pattern HTML_ANCHOR_PATTERN = Pattern.compile("<a href=\"(.*?)\">(.*?)</a>");
> + private static final String OPEN_BROWSER_WINDOW = "openWindowByUrl";
>
> private static class Linker extends BrowserFunction {
>
> @@ -143,6 +153,19 @@
>
> }
>
> + public class OpenWindowFunction extends BrowserFunction {
> +
> + public OpenWindowFunction (final Browser browser, final String name) {
> + super(browser, name);
> + }
> + public Object function (Object[] arguments) {
> + final String url = String.valueOf(arguments[0]);
> + final String title = String.valueOf(arguments[1]);
> + openBrowserByUrl(url, title);
> + return null;
> + }
> + }
> +
> private static class PageContainerResultProvider implements HtmlResultProvider {
> private IPageContainer editor;
>
> @@ -224,14 +247,64 @@
> private Collection<HtmlResultGroup> descriptors;
> private boolean isSinglePage = false;
>
> + private void openBrowserByUrl(final String url, final String title) {
> + final Display display = Display.getDefault();
> + final Shell shell = new Shell(display);
> + shell.setText(title);
> + shell.setLayout(new FillLayout());
> + final Browser browser = new Browser(shell, SWT.NONE);
> + initializeBrowser(display, browser, shell);
> + shell.open();
> + browser.setUrl(url);
> + }
> +
> + private void initializeBrowser(final Display display, final Browser browser, final Shell shell) {
> + browser.addOpenWindowListener(new OpenWindowListener() {
> + public void open(WindowEvent event) {
> + initializeBrowser(display, browser, shell);
> + event.browser = browser;
> + }
> + });
> + browser.addCloseWindowListener(new CloseWindowListener() {
> + public void close(WindowEvent event) {
> + Browser browser = (Browser)event.widget;
> + Shell shell = browser.getShell();
> + shell.close();
> + }
> + });
> + }
> +
> + /*
> + * We replace the anchors in the HTML when running in the JMC UI to make
> + * it possible to follow them. See JMC-5419 for more information.
> + */
> + private static String adjustAnchorFollowAction(String html) {
> + Map<String, String> map = new HashMap<>();
> + Matcher m = HTML_ANCHOR_PATTERN.matcher(html);
> + while (m.find()) {
> + map.put(m.group(1), m.group(2));
> + }
> + for(Map.Entry<String, String> e: map.entrySet()){
> + html = html.replace(e.getKey(), openWindowMethod(e.getKey(), e.getValue()));
> + }
> + return html;
> + }
> +
> + private static String openWindowMethod(String url, String name){
> + return new StringBuilder().append("#\" onclick=\"").append(OPEN_BROWSER_WINDOW).append("(").append("\u0027")
> + .append(url).append("\u0027").append(',').append("\u0027")
> + .append(name).append("\u0027").append(")").toString();
> + }
> +
> public ResultReportUi(boolean isSinglePage) {
> this.isSinglePage = isSinglePage;
> }
>
> public List<String> getHtml(IPageContainer editor) {
> List<String> overviewHtml = new ArrayList<>(1);
> - overviewHtml.add(RulesHtmlToolkit.generateStructuredHtml(new PageContainerResultProvider(editor), descriptors,
> + String adjustedHtml = adjustAnchorFollowAction(RulesHtmlToolkit.generateStructuredHtml(new PageContainerResultProvider(editor), descriptors,
> resultExpandedStates, true));
> + overviewHtml.add(adjustedHtml);
> return overviewHtml;
> }
>
> @@ -245,8 +318,10 @@
> .allMatch(d -> d > RulesHtmlToolkit.IN_PROGRESS && d < Severity.INFO.getLimit()) && !showOk;
> browser.evaluate(String.format("overview.allOk(%b);", allOk)); //$NON-NLS-1$
> } catch (SWTException swte) {
> - browser.setText(RulesHtmlToolkit.generateStructuredHtml(new PageContainerResultProvider(editor),
> - descriptors, resultExpandedStates, false));
> + String html = RulesHtmlToolkit.generateStructuredHtml(new PageContainerResultProvider(editor),
> + descriptors, resultExpandedStates, false);
> + String adjustedHtml = adjustAnchorFollowAction(html);
> + browser.setText(adjustedHtml);
> }
> }
> }
> @@ -276,7 +351,8 @@
> continue;
> }
> long score = Math.round(result.getScore());
> - String quoteEscape = RulesHtmlToolkit.getDescription(result).replaceAll("\\\"", "\\\\\""); //$NON-NLS-1$ //$NON-NLS-2$
> + String adjustedHtml = adjustAnchorFollowAction(RulesHtmlToolkit.getDescription(result));//$NON-NLS-1$ //$NON-NLS-2$
> + String quoteEscape = adjustedHtml.replaceAll("\\\"", "\\\\\""); //$NON-NLS-1$ //$NON-NLS-2$
> String description = quoteEscape.replaceAll("\n", "</br>"); //$NON-NLS-1$ //$NON-NLS-2$
> script.append(String.format("overview.updateResult(\"%s\", %d, \"%s\");", //$NON-NLS-1$
> result.getRule().getId(), score, description));
> @@ -310,9 +386,11 @@
> try {
> FlightRecorderUI.getDefault().getLogger().log(Level.INFO,
> "Could not update single result, redrawing html view. " + e.getMessage()); //$NON-NLS-1$
> - browser.setText(isSinglePage ? RulesHtmlToolkit.generateSinglePageHtml(results)
> + String html = isSinglePage ? RulesHtmlToolkit.generateSinglePageHtml(results)
> : RulesHtmlToolkit.generateStructuredHtml(new PageContainerResultProvider(editor),
> - descriptors, resultExpandedStates, false));
> + descriptors, resultExpandedStates, false);
> + String adjustedHtml = adjustAnchorFollowAction(html);
> + browser.setText(adjustedHtml);
> } catch (Exception e1) {
> FlightRecorderUI.getDefault().getLogger().log(Level.WARNING, "Could not update Result Overview", //$NON-NLS-1$
> e1);
> @@ -346,11 +424,13 @@
> String html = isSinglePage ? RulesHtmlToolkit.generateSinglePageHtml(results)
> : RulesHtmlToolkit.generateStructuredHtml(new PageContainerResultProvider(editor), descriptors,
> resultExpandedStates, false);
> - browser.setText(html, true);
> + String adjustedHtml = adjustAnchorFollowAction(html);
> + browser.setText(adjustedHtml, true);
> browser.setJavascriptEnabled(true);
> browser.addProgressListener(new ProgressAdapter() {
> @Override
> public void completed(ProgressEvent event) {
> + new OpenWindowFunction(browser, OPEN_BROWSER_WINDOW); //$NON-NLS-1$
> new Linker(browser, "linker", descriptors, editor); //$NON-NLS-1$
> new Expander(browser, "expander"); //$NON-NLS-1$
> browser.execute(String.format("overview.showOk(%b);", showOk)); //$NON-NLS-1$
>
>
>
>> On Sep 7, 2018, at 12:08 AM, Marcus Hirt <marcus.hirt at oracle.com> wrote:
>>
>> Hi Miro,
>>
>> Some comments:
>>
>> 1. I think perhaps a better name can be found for correctAnchor method.
>> The current name makes it feel like the anchor was incorrect to begin with -
>> but we're adjusting the Anchor open actions to make them work better when
>> running in the Eclipse RCP UI. Also, a comment with a reference to the
>> bug would be nice to have there.
>>
>> 2. Likewise with the correctedAnchor variables. Especially in the getHtml()
>> method - calling it correctedAnchor may be misleading, since it is the
>> entire overview html, with adjusted anchors.
>>
>> Otherwise it looks fine to me.
>>
>> Kind regards,
>> Marcus
>>
>> On 2018-09-06, 23:13, "jmc-dev on behalf of Miro Wengner" <jmc-dev-bounces at openjdk.java.net on behalf of miro.wengner at gmail.com> wrote:
>>
>> Hi Everyone,
>> here is my patch to not working links inside the UI.
>> I’ve tested links in Overview, Tabs. I’ve also generated JfrHTMLRuleReports.
>> Looks all works properly.
>>
>>
>> Kind Regards,
>> Miro
>>
>>
>>
>>
>>
>
>
>
>
>
More information about the jmc-dev
mailing list