[ https://issues.apache.org/jira/browse/QPID-7382?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15458595#comment-15458595 ]
Alex Rudyy edited comment on QPID-7382 at 9/2/16 1:55 PM: ---------------------------------------------------------- Lorenz, Here are my review comments. # The implemented changes look good to me. However, I am wondering would not it be better to move the html markup for download link and preview into a template rather then generating html on the flight? # It seems an existing defect but the content of dialog does not occupy 100% of the space. I think it can be fixed as part of this JIRA by committing the following change {code} broker-plugins/management-http/src/main/java/resources/showMessage.html @@ -18,7 +18,7 @@ <div class="dijitHidden"> <div data-dojo-type="dijit.Dialog" style="width:600px;" data-dojo-props="title:'View Message'" id="showMessage"> - <table style="border: 0;"> + <table style="border: 0; width:100%"> <tr style="margin-bottom: 4pt"> <td style="width: 10em; vertical-align: top"><span style="font-weight: bold;">Message Number:</span></td> <td><span class="message-id"></span></td> {code} # By clicking on download link the message content is replacing WMC UI. Although it is an existing issue, I think that it needs to be fixed to make content downloading user friendly by adding Content-Disposition header to the content, for example: {code} broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java @@ -2684,11 +2684,13 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>> public static final int UNLIMITED = -1; private final MessageReference<?> _messageReference; private final long _limit; + private final String _queueName; - public MessageContent(MessageReference<?> messageReference, long limit) + public MessageContent(MessageReference<?> messageReference, long limit, String queueName) { _messageReference = messageReference; _limit = (limit == UNLIMITED ? messageReference.getMessage().getSize() : limit); + _queueName = queueName; } @Override @@ -2735,6 +2737,12 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>> return _messageReference.getMessage().getMessageHeader().getEncoding(); } + @RestContentHeader("Content-Disposition") + public String getContentDisposition() + { + return "attachment; filename=\"" + _queueName + + "/" + _messageReference.getMessage().getMessageHeader() + "\""; + } } private static class AcquireAllQueueEntryFilter implements QueueEntryFilter @@ -3507,7 +3515,7 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>> visit(messageFinder); if(messageFinder.isFound()) { - return new MessageContent(messageFinder.getMessageReference(), limit); + return new MessageContent(messageFinder.getMessageReference(), limit, this.getName()); } else { {code} I think that 2 and 3 should be addressed whilst 1 is up to you to decide whether we can make code a bit nicer by moving markup into template. was (Author: alex.rufous): Lorenz, Here are my review comments. # The implemented changes look good to me. However, I am wondering would not it be easier to move the html markup for download link and preview into a template class rather then generating html on the flight? # It seems an existing defect but the content of dialog does not occupy 100% of the space. I think it can be fixed as part of this JIRA by committing the following change {code} broker-plugins/management-http/src/main/java/resources/showMessage.html @@ -18,7 +18,7 @@ <div class="dijitHidden"> <div data-dojo-type="dijit.Dialog" style="width:600px;" data-dojo-props="title:'View Message'" id="showMessage"> - <table style="border: 0;"> + <table style="border: 0; width:100%"> <tr style="margin-bottom: 4pt"> <td style="width: 10em; vertical-align: top"><span style="font-weight: bold;">Message Number:</span></td> <td><span class="message-id"></span></td> {code} # By clicking on download link the message content is replacing WMC UI. Although it is an existing issue, I think that it needs to be fixed to make content downloading user friendly by adding Content-Disposition header to the content, for example: {code} broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java @@ -2684,11 +2684,13 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>> public static final int UNLIMITED = -1; private final MessageReference<?> _messageReference; private final long _limit; + private final String _queueName; - public MessageContent(MessageReference<?> messageReference, long limit) + public MessageContent(MessageReference<?> messageReference, long limit, String queueName) { _messageReference = messageReference; _limit = (limit == UNLIMITED ? messageReference.getMessage().getSize() : limit); + _queueName = queueName; } @Override @@ -2735,6 +2737,12 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>> return _messageReference.getMessage().getMessageHeader().getEncoding(); } + @RestContentHeader("Content-Disposition") + public String getContentDisposition() + { + return "attachment; filename=\"" + _queueName + + "/" + _messageReference.getMessage().getMessageHeader() + "\""; + } } private static class AcquireAllQueueEntryFilter implements QueueEntryFilter @@ -3507,7 +3515,7 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>> visit(messageFinder); if(messageFinder.isFound()) { - return new MessageContent(messageFinder.getMessageReference(), limit); + return new MessageContent(messageFinder.getMessageReference(), limit, this.getName()); } else { {code} I think that 2 and 3 should be addressed whilst 1 is up to you to decide whether we can make code a bit nicer by moving markup into template. > Message dialogue tries to inline the text of a text message content > regardless of the its length > ------------------------------------------------------------------------------------------------ > > Key: QPID-7382 > URL: https://issues.apache.org/jira/browse/QPID-7382 > Project: Qpid > Issue Type: Bug > Components: Java Broker > Affects Versions: 0.30, 0.32, qpid-java-6.0 > Reporter: Keith Wall > Fix For: qpid-java-6.1, qpid-java-6.0.5 > > > When viewing a message through the WMC, if the message's mime type indicates > that the content it text, the message dialogue tries to inline the content, > properly escaping the content first. If the content is very large, this > approach is problematic and can cause formatting problems with buttons out of > reach, or at worst an unresponsive or crashed client side UI. > The WMC should display the leading portion of a long text message using > ellipses and offer a download button to retrieve the full text content. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org